Re: Compilation warnings in jbsockets.c
Ian Silvester <[email protected]>
| Newsgroups | gmane.comp.web.privoxy.devel |
|---|---|
| Message-ID | <[email protected]> |
On 2013-03-06, at 8:17 AM, Fabian Keil wrote: > Ian Silvester <[email protected]> wrote: > >> >> On 2013-03-05, at 6:16 AM, Fabian Keil wrote: >> >>> Ian Silvester <[email protected]> wrote: >>> >>>> Ever since I started handling the OS X builds I've had the following warnings when building (regardless of architecture being targeted): >>>> >>>> gcc -c -pipe -I/usr/local/include/ -isysroot /Developer/SDKs/MacOSX10.4u.sdk -arch ppc -mmacosx-version-min=10.4 -Dunix -Wall jbsockets.c -o jbsockets.o >>>> jbsockets.c: In function ‘get_host_information’: >>>> jbsockets.c:1063: warning: pointer targets in passing argument 3 of ‘getsockname’ differ in signedness >>>> jbsockets.c: In function ‘accept_connection’: >>>> jbsockets.c:1291: warning: pointer targets in passing argument 3 of ‘accept’ differ in signedness >>>> >>>> I've kept meaning to post them in case its important that the code >>>> is modified to avoid them, though it doesn't appear to be important to me. >>> >>> In both cases the type of the third arguments depends on whether >>> or not __APPLE_CC__ is defined because apparently that was useful >>> in 2002. >>> >>> If your gcc defines it, >> >> It does. >> >>> you could try removing the two checks >>> to get a proper socklen_t. If it doesn't, you could add a define >>> to "downgrade" the type to int. >> >> So I removed "|| __APPLE_CC__" from the two ifdefs and indeed the warnings are no longer displayed. > > Great, please commit. > >>> I agree that these warnings look harmless, but if they can be >>> trivially fixed we should probably do it. >> >> Are you happy that I commit the file with the removals or should >> I test the installed build first. If I test, what should I be testing for? > > In theory all commits should be tested. If the commit doesn't > affect the installation process, testing the binary without a > complete re-installation should be sufficient. Dropped in the replacement binary. > In this case you could verify that http://p.p/ still shows the > same "This is Privoxy ..." message It does. > and that the regression tests still pass. For some reason curl cannot resolve p.p/ (and hence privoxy-regression-test.pl fails) though the browsers can. The last time I ran the regression tests I didn't run into this issue and my network and Privoxy configuration is unchanged apart from going to 3.0.21's code which I sincerely doubt is the cause. It's not down to you to fix my configuration but do you have any pointers? Ian > > Fuzzing around with gdb to reach the various error conditions > and edge cases never hurts, but for a trivial commit like this > I wouldn't do it myself. > > Removing the defines should only affect OS X and as you enable > the code paths that have been used and tested on other platforms > for years the risk seems pretty low to me. > > Fabian > ------------------------------------------------------------------------------ > Symantec Endpoint Protection 12 positioned as A LEADER in The Forrester > Wave(TM): Endpoint Security, Q1 2013 and "remains a good choice" in the > endpoint security space. For insight on selecting the right partner to > tackle endpoint security challenges, access the full report. > http://p.sf.net/sfu/symantec-dev2dev_______________________________________________ > Ijbswa-developers mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/ijbswa-developers ------------------------------------------------------------------------------ Symantec Endpoint Protection 12 positioned as A LEADER in The Forrester Wave(TM): Endpoint Security, Q1 2013 and "remains a good choice" in the endpoint security space. For insight on selecting the right partner to tackle endpoint security challenges, access the full report. http://p.sf.net/sfu/symantec-dev2dev