This is the mail archive of the cygwin-patches mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix misc aliasing warnings.


Corinna Vinschen wrote:

> Concerning fstat_helper, I don't like to slip another layer into these
> calls to pamper an anal-retentive compiler.  I would rather like to fix
> this by removing the FILETIME type from the affected places and use
> LARGE_INTEGER throughout.  It's not overly tricky, given that FILETIME
> time == LARGE_INTEGER kernel time.

  I'll give that patch you posted a test.

>> -	  di = &((DISK_GEOMETRY_EX *) dbuf)->Geometry;
>> +	  DISK_GEOMETRY_EX *dgx = (DISK_GEOMETRY_EX *) dbuf;
>> +	  di = &dgx->Geometry;
> 
> That's ok, even though I don't understand what gcc has to grouch about
> it.  The expressions should be identical.

  I don't always understand the aliasing rules.  Maybe it's the introduction
of a sequence point between the cast and the dereference that pacifies it.

> Shouldn't defining szBuffer as a union pointer avoid the need to memcpy?
> Like this:

  Well yeh, but it's a far bigger change, which was why I didn't.  Don't mind
giving it a test though.

> Wouldn't temporary pointers
> avoid the memcpy?

  Probably, but I was expecting the compiler to thoroughly optimize those
memcpys anyway.  I'll double-check with this and the previous one how the
generated code looks.

>>  #define IN6_ARE_ADDR_EQUAL(a, b) \
>> -	(((const uint32_t *)(a))[0] == ((const uint32_t *)(b))[0] \
>> -	 && ((const uint32_t *)(a))[1] == ((const uint32_t *)(b))[1] \
>> -	 && ((const uint32_t *)(a))[2] == ((const uint32_t *)(b))[2] \
>> -	 && ((const uint32_t *)(a))[3] == ((const uint32_t *)(b))[3])
>> +	(!memcmp ((a), (b), 4 * sizeof (uint32_t)))
> 
> Hang on.  That's almost exactly the definition of IN6_ARE_ADDR_EQUAL as
> on Linux and on other systems.  If that doesn't work anymore, not only
> this one has to be changed, but all the equivalent expressions
> throughout netinet/in.h.  The gcc guys aren';t serious about that,
> are they?

  I'll ask upstream about this one, and perhaps the disk geometry thing as
well.  It's not like any of this is urgent, 4.5 is still a little ways off
release so I'm just trying to lay some groundwork in advance.

  So, I'll test your version of the fhandler patch, and consult upstream about
a couple of the others, and we'll come back to this shortly.  Thanks for the
reviews.

    cheers,
      DaveK


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]