Hi John, Well, slpd_knownda.c:564 was probably the worst example I could have given ;-) I agree that this line is superfluous.
A better example is perhaps slpd_regfile.c:506 where perhaps the compiler can leave out the assignment completely. So some lines could simply be removed, but other situations are perhaps a real problem. Below are the warnings 'make' gives on my system. BR, Roel PS. I already tried to send a mail with the full 'make' output as an attachment, but that mail was blocked by mailman, awaiting your approval... slp_dhcp.c:343: warning: dereferencing pointer ‘sin’ does break strict-aliasing rules slp_filter_l.l:175: warning: cast to pointer from integer of different size slp_filter_l.l:182: warning: cast from pointer to integer of different size slp_filter_l.c:1316: warning: ‘yyunput’ defined but not used slp_filter_l.c:1357: warning: ‘input’ defined but not used slp_attr_l.l:220: warning: cast from pointer to integer of different size slp_attr_l.l:226: warning: cast to pointer from integer of different size slp_attr_l.c:1381: warning: ‘yyunput’ defined but not used slp_attr_l.c:1422: warning: ‘input’ defined but not used slpd_incoming.c:860: warning: dereferencing pointer ‘myaddr.53’ does break strict-aliasing rules slpd_knownda.c:557: warning: dereferencing pointer ‘daaddr.51’ does break strict-aliasing rules slpd_knownda.c:564: warning: dereferencing pointer ‘daaddr.53’ does break strict-aliasing rules slpd_regfile.c:506: warning: dereferencing pointer ‘peer.33’ does break strict-aliasing rules slpd_socket.c:239: warning: dereferencing pointer ‘addr.34’ does break strict-aliasing rules slpd_socket.c:299: warning: dereferencing pointer ‘loaddr.40’ does break strict-aliasing rules slpd_socket.c:300: warning: dereferencing pointer ‘loaddr.40’ does break strict-aliasing rules slpd_socket.c:309: warning: dereferencing pointer ‘loaddr.41’ does break strict-aliasing rules slpd_socket.c:312: warning: dereferencing pointer ‘loaddr.41’ does break strict-aliasing rules libslp_knownda.c:542: warning: dereferencing pointer ‘peeraddr.47’ does break strict-aliasing rules On 05/27/2011 07:54 AM, John Calcote wrote: > Hi Roel, > > Looking at slpd_knownda.c:564, I have to say I'm a little confused as to why > this line of code even exists. It appears to be a duplicate of the line > above it using an alias. As I started to say in my previous message, > sockaddr was originally a generic type large enough to hold all known > address types at the time it was invented. Today, with many more address > types available, a new generic buffer type is needed - that's why they > invented sockaddr_storage - I believe it will hold addresses up to 128 bytes > long. > > All of the sockaddr family of structures share a common prefix - a 16-bit > value called "family". In fact, in the latest gcc header files, this common > prefix is defined within each address structure using a single macro that > names the common "family" field according to the address type - something > like "unsigned short<type>_family". For sockaddr_storage, the prefix ss is > used for<type>, so the field becomes ss_family, but in ipv4 addresses using > sockaddr_in, the prefix is sin, so the field is named sin_family. > > In other words, line 563 sets the address family in the sockaddr_storage > address buffer to AF_INET6, then in the very next line, the same value is > written over the same location - or rather, that is apparently the author's > intent, but given strict aliasing, the compiler is allowed to assume that > the location referred to by lines 563 and 564 are not the same because > they're referred to with pointers of different types. In fact, this is > harmless because the strict-aliasing rules only affect things like variable > hoisting (where variables assigned within a loop may be hoisted out of the > loop for performance's sake if the compiler can detect that the assignment > is redundant - with strict aliasing rules, the compiler is allowed to make > much broader assumptions). > > I think we should rework this code a bit - remove the extra type-casted > assignments - they really are totally redundant, as far as I can tell. > > John > > -----Original Message----- > From: Roel van de Kraats [mailto:rkra...@dds.nl] > Sent: Wednesday, May 25, 2011 1:03 PM > To: John Calcote > Cc: openslp-devel@lists.sourceforge.net > Subject: Re: [Openslp-devel] Bug in libslp_knownda.c > > Thanks for clarifying this, John. > > Although everything seems to work fine, I'm a bit worried about the > 'strict-aliasing' warnings my gcc compiler gives, like this one: > > slpd_knownda.c:564: warning: dereferencing pointer 'daaddr.53' does break > strict-aliasing rules > > With the used optimization levels, it is possible that modifying an 'alias' > is optimized away completely, since the compiler assumes its value isn't > used anymore. See e.g. > http://cellperformance.beyond3d.com/articles/2006/06/understanding-strict-al > iasing.html > > We could use the '-fno-strict-aliasing' compiler option, but I'd prefer to > use a 'cleaner' way. I'm not sure if a 'double cast' through void* is good > enough. > > BR, > Roel > > On 05/25/2011 05:25 PM, John Calcote wrote: >> This has been a minor sore point for decades in the socket interface - >> how to refer to a structure that could hold any of a number of >> different address types. Early on, it was decided just to refer to a >> sockaddr object - an object of a well-known type and size - too small >> these days, but at least useful for ipv4 addresses. This was done >> because void* was not actually added to the C standard till 1989 when >> the C89 standard came out. Prior to that, your only option was to > typecast. >> In short, this is exactly the sort of thing for which void * was >> originally introduced. You pass a generic pointer to the data >> structure, along with a type discriminator and a buffer length. In >> this case, the type discriminator is always at a well-known offset >> within the buffer referred to, and the size is a separate argument. >> Had void* been available when the sockets interface was being >> designed, I believe they would have used it instead of passing >> sockaddr pointers around, because in the C language, you can >> automatically cast to and from void (not so in C++, of course, but not >> relevant here), so you can simply use a void* as if it were any of the > sockaddr family of structures. >> Is it type safe? Of course not. You can just as easily auto-cast it to >> the wrong type of thing. But given the fact that the original socket >> spec has you manually type-casting from one type of structure to >> another all over the place, it's at least as safe, but much more > convenient. >> John >> >> -----Original Message----- >> From: Roel van de Kraats [mailto:rkra...@dds.nl] >> Sent: Wednesday, May 25, 2011 6:06 AM >> To: 'openslp-devel@lists.sourceforge.net' >> Subject: Re: [Openslp-devel] Bug in libslp_knownda.c >> >> >> >> On 05/25/2011 01:18 PM, Morrell Richard (external) wrote: >>> Hi Roel, >>> >>> The change in r1597 was only supposed to abstract the cache refresh >>> code into a separate function, not change anything else, so the >>> change to the address size in the second call was incorrect. Please >>> go ahead with the update. >> Done. See r1673. >> >> In my opinion it is a bit 'dangerous' to pass through void pointers >> like this. Wouldn't it be better to explicitly use 'struct > sockaddr_storage' >> pointers? Or even use a union throughout the whole source tree to hold >> the different 'sockaddr' types, like >> http://svn.tartarus.org/sgt/putty/unix/uxnet.c?p2=%2Fputty%2Funix%2Fux >> net.c& >> p1=putty%2Funix%2Fuxnet.c&r1=8612&r2=8611&view=diff&pathrev=8612 >> ? >> >> BR, >> Roel >>> Regards, >>> >>> Richard >>> >>> -----Original Message----- >>> From: Roel van de Kraats [mailto:rkra...@dds.nl] >>> Sent: 25 May 2011 11:34 >>> To: 'openslp-devel@lists.sourceforge.net' >>> Subject: [Openslp-devel] Bug in libslp_knownda.c >>> >>> >>> Dear all, >>> >>> It seems that libslp_knownda.c has a bug on line 716 (see >>> >> http://openslp.svn.sourceforge.net/viewvc/openslp/trunk/openslp/libslp >> /libsl >>> p_knownda.c?view=markup&pathrev=1672), >>> because sizeof(struct in_addr) is too small to copy the whole address. >>> This causes 'slptool findsrvtypes' to fail in case another host acts >>> as DA. This was introduced in r1597 (see >>> >> http://openslp.svn.sourceforge.net/viewvc/openslp?view=revision&revisi >> on=159 >>> 7). >>> >>> I guess it is safe to replace 'sizeof (struct in_addr)' with >>> 'sizeof(struct sockaddr_storage)', but I'm not sure. If anyone can >>> confirm this, I'll commit the fix. >>> >>> BR, >>> Roel >>> >>> >>> >>> >> ---------------------------------------------------------------------- >> ------ >>> -- >>> vRanger cuts backup time in half-while increasing security. >>> With the market-leading solution for virtual backup and recovery, you >>> get blazing-fast, flexible, and affordable data protection. >>> Download your free trial now. >>> http://p.sf.net/sfu/quest-d2dcopy1 >>> _______________________________________________ >>> Openslp-devel mailing list >>> Openslp-devel@lists.sourceforge.net >>> https://lists.sourceforge.net/lists/listinfo/openslp-devel >>> >>> This email, including any attachment, is a confidential communication >>> intended solely for the use of the individual or entity to whom it is >>> addressed. It contains information which is private and may be >>> proprietary or covered by legal professional privilege. If you have >>> received this >> email >>> in error, please notify the sender upon receipt, and immediately >>> delete it from your system. >>> >>> Anything contained in this email that is not connected with the >>> businesses of this company is neither endorsed by nor is the >>> liability of this >> company. >>> Whilst we have taken reasonable precautions to ensure that any >>> attachment >> to >>> this email has been swept for viruses, we cannot accept liability for >>> any damage sustained as a result of software viruses, and would >>> advise that >> you >>> carry out your own virus checks before opening any attachment. >>> >>> >>> >> ---------------------------------------------------------------------- >> ------ >> -- >>> vRanger cuts backup time in half-while increasing security. >>> With the market-leading solution for virtual backup and recovery, you >>> get blazing-fast, flexible, and affordable data protection. >>> Download your free trial now. >>> http://p.sf.net/sfu/quest-d2dcopy1 >>> _______________________________________________ >>> Openslp-devel mailing list >>> Openslp-devel@lists.sourceforge.net >>> https://lists.sourceforge.net/lists/listinfo/openslp-devel >> ---------------------------------------------------------------------- >> ------ >> -- >> vRanger cuts backup time in half-while increasing security. >> With the market-leading solution for virtual backup and recovery, you >> get blazing-fast, flexible, and affordable data protection. >> Download your free trial now. >> http://p.sf.net/sfu/quest-d2dcopy1 >> _______________________________________________ >> Openslp-devel mailing list >> Openslp-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/openslp-devel >> > ------------------------------------------------------------------------------ Simplify data backup and recovery for your virtual environment with vRanger. Installation's a snap, and flexible recovery options mean your data is safe, secure and there when you need it. Data protection magic? Nope - It's vRanger. Get your free trial download today. http://p.sf.net/sfu/quest-sfdev2dev _______________________________________________ Openslp-devel mailing list Openslp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openslp-devel
