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

[John Carter]

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);


On Mon, 2003-05-12 at 20:51, Richard Frith-Macdonald wrote:
> http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=9751
> 
> I was just looking at this ... and I don't think this is a bug.
> If I understand the code correctly, it is removing a single byte (the 
> garbage collecting invisibility marker) from the type string.  So the 
> length of the new string is one byte less than that of the original.
> So allocating strlen(ivar->ivar_type) bytes is correct.
> It might perhaps be worth adding a comment to thiks effect in the source 
> though.