From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14751 invoked by alias); 3 Jun 2013 10:31:17 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 14739 invoked by uid 89); 3 Jun 2013 10:31:17 -0000 X-Spam-SWARE-Status: No, score=-7.8 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS,TW_XS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Mon, 03 Jun 2013 10:31:16 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r53AVFds006067 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 3 Jun 2013 06:31:15 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r53AVDFO002720; Mon, 3 Jun 2013 06:31:14 -0400 Message-ID: <51AC7071.6090502@redhat.com> Date: Mon, 03 Jun 2013 10:31:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 MIME-Version: 1.0 To: Jan Kratochvil , gdb-patches@sourceware.org Subject: Re: [RFA 5/7 take 4] Improved linker-debugger interface References: <20130516144340.GA2105@blade.nx> <20130516144838.GF2105@blade.nx> <20130517185002.GA10447@host2.jankratochvil.net> <20130524083044.GC4602@blade.nx> <51A64E24.2020901@redhat.com> <20130530104346.GA6217@blade.nx> <51A789CF.2000302@redhat.com> <20130531132206.GB7409@blade.nx> In-Reply-To: <20130531132206.GB7409@blade.nx> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-06/txt/msg00007.txt.bz2 On 05/31/2013 02:22 PM, Gary Benson wrote: > Pedro Alves wrote: >> On 05/30/2013 11:43 AM, Gary Benson wrote: >>> + char name[32] = { '\0' }; >>> + >>> + /* Fedora 17 and Red Hat Enterprise Linux 6.2-6.4 >>> + shipped with an early version of the probes code in >>> + which the probes' names were prefixed with "rtld_" >>> + and the "map_failed" probe did not exist. The >>> + locations of the probes are otherwise the same, so >>> + we check for probes with prefixed names if probes >>> + with unprefixed names are not present. */ >>> + >>> + if (with_prefix) >>> + strncat (name, "rtld_", sizeof (name) - strlen (name) - 1); >>> + >>> + strncat (name, probe_info[i].name, >>> + sizeof (name) - strlen (name) - 1); >> >> BTW, I'd rather this was written as something like: >> >> char name[32]; >> >> if (with_prefix) >> xsnprintf (name, sizeof (name), "rtld_%s", probe_info[i].name); >> else >> xsnprintf (name, sizeof (name), "%s", probe_info[i].name); >> >> >> strncat like that is really not future proof, as we'll just get get >> a silently truncated string if a new probe appears with a name that >> is too big. xsnprintf OTOH gdb_asserts the string fits in the >> buffer. Granted, whoever added such a probe would probably notice >> this, but it's a style best avoided. (Witness how so few strncat >> calls there are in gdb. There are more strcat calls, but that only >> shows that whenever we _do_ care about string overruns, we tend to >> use something else, not strncat.) >> Plus, it looks more readable to me that way. :-) >> Alternatively, we could add a xstrncat that asserts truncation >> never happens, though xsnprintf in this case still looks a bit >> more readable to me :-) > > How about this: > > const char *name = probe_info[i].name; > char buf[32]; > > /* Fedora 17 and Red Hat Enterprise Linux 6.2-6.4 > shipped with an early version of the probes code in > which the probes' names were prefixed with "rtld_" > and the "map_failed" probe did not exist. The > locations of the probes are otherwise the same, so > we check for probes with prefixed names if probes > with unprefixed names are not present. */ > if (with_prefix) > { > xsnprintf (buf, sizeof (buf), "rtld_%s", name); > name = buf; > } > > Revised patch attached. That's great, thanks. -- Pedro Alves