From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7160 invoked by alias); 13 May 2003 05: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 7118 invoked by uid 71); 13 May 2003 05:06:01 -0000 Date: Tue, 13 May 2003 05:06:00 -0000 Message-ID: <20030513050601.7103.qmail@sources.redhat.com> To: nobody@gcc.gnu.org Cc: gcc-prs@gcc.gnu.org, From: Richard Frith-Macdonald Subject: Re: libobjc/9751: malloc of strlen, not strlen+1 Reply-To: Richard Frith-Macdonald X-SW-Source: 2003-05/txt/msg01418.txt.bz2 List-Id: The following reply was made to PR libobjc/9751; it has been noted by GNATS. From: Richard Frith-Macdonald To: John Carter 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 06:02:20 +0100 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.