[Asterisk-Dev] FW: asterisk/doc CODING-GUIDELINES,1.4,1.5

[Kevin P. Fleming]

Michael Giagnocavo wrote:

>>+ if (strlen(name)) {
>>+  newname = alloca(strlen(name));
>>+  strncpy(newname, name, strlen(name);
>>+ }
>>+  
>>+vs
>>+
>>+ if((len = strlen(name))) {
>>+  newname = alloca(len);
>>+  strncpy(newname, name, len);
>>+ }
> 
> 
> Maybe they should point out that you need to make the memory versus CPU
> cycles after you calculate the hit... i.e., using a nice amount of memory to
> store a value that's easily computed is MUCH worse than wasting a few
> hundred cycles, since one cache miss is gonna knock you back a lot...

To say nothing of the fact if you look at the generated object code for 
the above sequences (as long as it was compiled with -O2 or above), it 
will very likely be identical. GCC's compiler is very aware of things 
like this, and since it knows that strlen is "pure" (its result depends 
only on its input) then it will not bother to call strlen twice in this 
situation (it will probably call it once and keep the result in a register).

Finally, it's silly to use strlen at all here for the first check, 
that's why we have ast_strlen_zero, which checks only the first 
character of the string to see whether it's null or not... however, that 
depends on how often the string being checked here will be non-empty; if 
it will be empty a great deal of the time, ast_strlen_zero will be a big 
CPU time savings. If on the other hand it will be non-empty most of the 
time, calling strlen will be OK, since we need that value anyway.

It's also a waste of CPU time to use strncpy here (or even strcpy for 
that matter), as we know for a fact that the destination buffer is 
adequately sized to hold the string, since we just allocated it. strncpy 
  will be checking after it copies each character to see if it has 
filled the buffer, which is pointless since it will not. In this case I 
would either use strcpy(newname, name) or just memcpy(newname, name, len 
+ 1). strncpy should only be used when you do not know for sure that the 
destination buffer is large enough for the input string, which is the 
case when you are using statically-allocated buffers. If you are using a 
dynamically-allocated buffer and you didn't make it large enough for the 
string, you are doing something wrong :-)

In other words, generalizations like this are just that: 
generalizations. If you are writing code, you should be very aware of 
how it will operate in its likely environment, and what the compiler is 
going to do with the code you write when it gets optimized.

(and that's ignoring the fact that the first example won't even compile 
due to a syntax error <G>)