From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Ku2nU-0002IT-7h for qemu-devel@nongnu.org; Sun, 26 Oct 2008 06:24:36 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Ku2nT-0002Hl-Gi for qemu-devel@nongnu.org; Sun, 26 Oct 2008 06:24:35 -0400 Received: from [199.232.76.173] (port=34897 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Ku2nT-0002Hi-2I for qemu-devel@nongnu.org; Sun, 26 Oct 2008 06:24:35 -0400 Received: from rv-out-0708.google.com ([209.85.198.247]:30524) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Ku2nS-0003Ps-PE for qemu-devel@nongnu.org; Sun, 26 Oct 2008 06:24:35 -0400 Received: by rv-out-0708.google.com with SMTP id f25so1460029rvb.22 for ; Sun, 26 Oct 2008 03:24:33 -0700 (PDT) Message-ID: Date: Sun, 26 Oct 2008 12:24:33 +0200 From: "andrzej zaborowski" Subject: Re: [Qemu-devel] [5532] Replace uses of strndup (a GNU extension) with Qemu pstrdup In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org 2008/10/26 Blue Swirl : > On 10/25/08, andrzej zaborowski wrote: >> 2008/10/25 Blue Swirl : >> > On 10/25/08, andrzej zaborowski wrote: >> >> 2008/10/25 Blue Swirl : >> >> > On 10/25/08, andrzej zaborowski wrote: >> >> >> 2008/10/25 Blue Swirl : >> >> >> > On 10/25/08, andrzej zaborowski wrote: >> >> >> >> 2008/10/25 Blue Swirl : >> >> >> >> >> >> >> >> > Revision: 5532 >> >> >> >> > http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5532 >> >> >> >> > Author: blueswir1 >> >> >> >> > Date: 2008-10-25 11:23:27 +0000 (Sat, 25 Oct 2008) >> >> >> >> > >> >> >> >> > Log Message: >> >> >> >> > ----------- >> >> >> >> > Replace uses of strndup (a GNU extension) with Qemu pstrdup >> >> >> >> > >> >> >> >> > Modified Paths: >> >> >> >> > -------------- >> >> >> >> > trunk/cutils.c >> >> >> >> > trunk/hw/bt-hci.c >> >> >> >> > trunk/qemu-common.h >> >> >> >> > >> >> >> >> > Modified: trunk/cutils.c >> >> >> >> > =================================================================== >> >> >> >> > --- trunk/cutils.c 2008-10-25 11:21:28 UTC (rev 5531) >> >> >> >> > +++ trunk/cutils.c 2008-10-25 11:23:27 UTC (rev 5532) >> >> >> >> > @@ -50,6 +50,18 @@ >> >> >> >> > return buf; >> >> >> >> > } >> >> >> >> > >> >> >> >> > +/* strdup with a limit */ >> >> >> >> > +char *pstrdup(const char *str, size_t buf_size) >> >> >> >> > +{ >> >> >> >> > + size_t len; >> >> >> >> > + char *buf; >> >> >> >> > + >> >> >> >> > + len = MIN(buf_size, strlen(str)); >> >> >> >> > + buf = qemu_malloc(len); >> >> >> >> > + pstrcpy(buf, len, str); >> >> >> >> > + return buf; >> >> >> >> > +} >> >> >> >> >> >> >> >> >> >> >> >> I think here also pstrcpy will only copy up to buf_size - 1 characters >> >> >> >> while strndup would copy buf_size chars. >> >> >> > >> >> >> > That is actually safer if we always want the strings to be NUL terminated. >> >> >> >> >> >> >> >> >> strndup also always NUL terminates the string so it's just as safe, >> >> >> the length is just different. >> >> >> >> >> >> >> >> >> > >> >> >> > But the allocation length is wrong, it should be MIN(buf_size, strlen(str) + 1). >> >> >> >> >> >> >> >> >> By my reading of the manual, it should rather be MIN(buf_size, strlen(str)) + 1. >> >> > >> >> > But then the length could be incorrect: buf_size + 1. >> >> >> >> >> >> That's what it should be in case lmp_name is 248 chars long. I think >> >> the confusion is because you called the parameter buf_size while >> >> strndup calls it n (the number of characters.. not buffer size). >> > >> > If strlen(lmp_name) >= 248, only first 247 should be copied and the >> > final character should be NUL, because sizeof(read_local_name_rp.name) >> > == buf_size == 248. >> >> >> Nope, actually now I notice that I assumed that strndup never executes >> strlen() on the parameter... I think it's a sane assumption (?). > > Yes, glibc uses strnlen: > http://sources.redhat.com/cgi-bin/cvsweb.cgi/libc/string/strndup.c?rev=1.11&content-type=text/x-cvsweb-markup&cvsroot=glibc Ah, cool. > >> read_local_name_rp.name format is specified by the bluetooth "Volume 2 >> Core system package, part E - Host Controller Interface Functional >> Specification" on pg. 394: >> >> 6.24 LOCAL NAME >> <...> >> If the name contained in the parameter is shorter than 248 octets, the >> end of the name is indicated by a NULL octet (0x00), and the following >> octets (to fill up 248 octets, which is the length of the parameter) do not >> have valid values. > > So, in this case neither strndup nor pstrdup are correct, because you > do not want the string to be NUL terminated in the strlen(lmp_name) >= > 248 case? PARAM(change_local_name, name) format is defined by the specification, but hci->device.lmp_name was just a C string, so that it could be used for example with printf in qemu. strndup converted it from one format to the other so it was correct. strncpy converted it back. Cheers