From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24595 invoked by alias); 12 May 2003 22:06:01 -0000 Mailing-List: contact gcc-prs-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Archive: List-Post: List-Help: Sender: gcc-prs-owner@gcc.gnu.org Received: (qmail 24579 invoked by uid 71); 12 May 2003 22:06:01 -0000 Date: Mon, 12 May 2003 22:06:00 -0000 Message-ID: <20030512220601.24573.qmail@sources.redhat.com> To: nobody@gcc.gnu.org Cc: gcc-prs@gcc.gnu.org, From: John Carter Subject: Re: libobjc/9751: malloc of strlen, not strlen+1 Reply-To: John Carter X-SW-Source: 2003-05/txt/msg01381.txt.bz2 List-Id: The following reply was made to PR libobjc/9751; it has been noted by GNATS. From: John Carter To: Richard Frith-Macdonald Cc: gcc-prs@gcc.gnu.org, gcc-bugs@gcc.gnu.org, gcc-gnats@gcc.gnu.org Subject: Re: libobjc/9751: malloc of strlen, not strlen+1 Date: Tue, 13 May 2003 09:56:14 +1200 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.