libobjc/9751: malloc of strlen, not strlen+1

[Richard Frith-Macdonald]

On Monday, May 12, 2003, at 10:56  pm, John Carter wrote:

> Hmm, looking at it again I still don't like it.
>
> If strncpy terminates due to having copied its "n" characters, it
> _doesn't_ copy in the null. (Yip, check the libc info page, as I say,
> the strncpy semantics are plain fugly and almost always doesn't do what
> you want...)
>
> The very next line uses strcat, which _expects_ a properly null
> terminated string! I can't believe this bit of code is reliable.
>
> In fact I will state a categorical principle any...
>   strncpy( blah, bloo, fishpaste);
> Followed by immediately by...
>   strwhateverlibcthing( blah,....);
> Can only work by accident!
>
> This is the code from gcc-3.2.3...
> 	  /* The variable is gc_invisible and we have to reverse it */
> 	  new_type = objc_atomic_malloc (strlen (ivar->ivar_type));
> 	  strncpy (new_type, ivar->ivar_type,
> 		   (size_t)(type - ivar->ivar_type));
> 	  strcat (new_type, type + 1);
> 	  ivar->ivar_type = new_type;
>
> I would rewrite that as...
>   size_t len = type - ivar->ivar_type;
>   new_type=objc_atomic_malloc(strlen(ivar-ivar_type));
>   memcpy( new_type, ivar->ivar_type, len);
>   strcpy( new_type+len, type+1);

So the size of the memory allocated is correct, but the use of
the strcat() is wrong... should have been strcpy().

I'd agree with your rewriting ... except for the typo in the argument
to strlen() and the improper to use of whitespace (as far as gnu
coding standards are concerned) of course :-)

There is no functional difference between strncpy() and memcpy()
in this case, but the memcpy() should be marginally faster.