[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #1943 [Tor Relay]: Helper functions {get, set}_uint{16, 32, 64}() are not used

#1943: Helper functions {get,set}_uint{16,32,64}() are not used
--------------------------------+-------------------------------------------
 Reporter:  asn                 |        Type:  defect   
   Status:  needs_review        |    Priority:  normal   
Milestone:  Tor: 0.2.2.x-final  |   Component:  Tor Relay
  Version:                      |    Keywords:  easy     
   Parent:                      |  
--------------------------------+-------------------------------------------

Comment(by nickm):

 a, b) Yes.  The objects in question are real honest-to-goodness uint32_ts
 or doubles.  Without loss of genearality, any double that gets actually
 declared as part of a struct, or on the stack, or wherever, can give you
 an honest-to-goodness double * when you take its address.  Even if you
 cast the pointer to a void*, it is still "really" a pointer to a double,
 and even the most whacko of interpretations of C99 let you cast it back to
 a double* and dereference it.

 The place where you get on thin ice is when you cast a void* to a double*,
 and that void * didn't start life as a double*.

 (malloc and its friends are a special case.  The output of malloc() is
 required to be an okay alignment for absolutely anything.)

 c) good idea.

 d) I think the old code is safe currently on platforms we care about given
 how our compiler work now, but I'm not willing to call them "safe" in the
 general case....

 But you shouldn't really worry about performance here; the compiler is
 very smart.  Go on, try it: write a quick little test function like:
 {{{
 #include <string.h>
 void set_to_short(char*out, unsigned short s)
 {
   memcpy(out, &s, 2);
 }
 }}}

 and compile it to assembly with gcc -Wall -O2 -s foo.c, and you'll get
 something like the following in foo.s:
 {{{
 _set:
         pushl   %ebp
         movl    %esp, %ebp
         subl    $4, %esp
         movl    12(%ebp), %edx
         movl    8(%ebp), %eax
         movw    %dx, (%eax)
         leave
         ret
 }}}

 Note that there is no call to memcpy() here; the compiler is smart enough
 to notice that it is allowed to turn the memcpy into a single movw
 instruction.

 I'm glad to merge this, but is there more?  The commit msg implies more is
 coming.

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/1943#comment:4>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs