New ARI warning Tue Nov 6 01:58:48 UTC 2012

New ARI warning Tue Nov 6 01:58:48 UTC 2012

[GDB Administrator]

281a282,283
> gdb/dwarf2read.c:8612: regression: xasprintf: Do not use xasprintf(), instead use xstrprintf
gdb/dwarf2read.c:8612:  xasprintf (&virtual_dwo_name, ',
> gdb/dwarf2read.c:8984: regression: xasprintf: Do not use xasprintf(), instead use xstrprintf
gdb/dwarf2read.c:8984:  xasprintf (&dwp_name, ', dwarf2_per_objfile->objfile->name);

[RFA] Fix New ARI warning Tue Nov 6 01:58:48 UTC 2012

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de GDB Administrator
> Envoyé : mardi 6 novembre 2012 02:59
> À : gdb-patches@sourceware.org
> Objet : New ARI warning Tue Nov 6 01:58:48 UTC 2012
> 
> 281a282,283
> > gdb/dwarf2read.c:8612: regression: xasprintf: Do not use xasprintf(),
> instead use xstrprintf
> gdb/dwarf2read.c:8612:  xasprintf (&virtual_dwo_name, ',
> > gdb/dwarf2read.c:8984: regression: xasprintf: Do not use xasprintf(),
> instead use xstrprintf
> gdb/dwarf2read.c:8984:  xasprintf (&dwp_name, ', dwarf2_per_objfile-
> >objfile->name);

Doug, 

could you check the patch below
which removes the new ARI warnings about use of 
xasprintf function in the commit

http://sourceware.org/ml/gdb-cvs/2012-11/msg00027.html



Pierre Muller

ChangeLog entry:
2012-11-06  Pierre Muller  <muller@sourceware.org>

        ARI xasprintf rule fixes.
        * dwarf2read.c (create_dwo_in_dwp): Use xstrprintf function
        instead of xasprintf.
        (open_and_init_dwp_file): Ditto.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.710
diff -u -p -r1.710 dwarf2read.c
--- dwarf2read.c        5 Nov 2012 15:50:21 -0000       1.710
+++ dwarf2read.c        6 Nov 2012 11:00:17 -0000
@@ -8609,13 +8609,14 @@ create_dwo_in_dwp (struct dwp_file *dwp_
      (fewer struct dwo_file objects to allocated).  Remember that for
really
      large apps there can be on the order of 8K CUs and 200K TUs, or more.
*/

-  xasprintf (&virtual_dwo_name, "virtual-dwo/%d-%d-%d-%d",
-            sections.abbrev.asection ? sections.abbrev.asection->id : 0,
-            sections.line.asection ? sections.line.asection->id : 0,
-            sections.loc.asection ? sections.loc.asection->id : 0,
-            (sections.str_offsets.asection
-             ? sections.str_offsets.asection->id
-             : 0));
+  virtual_dwo_name =
+    xstrprintf ("virtual-dwo/%d-%d-%d-%d",
+               sections.abbrev.asection ? sections.abbrev.asection->id : 0,
+               sections.line.asection ? sections.line.asection->id : 0,
+               sections.loc.asection ? sections.loc.asection->id : 0,
+               (sections.str_offsets.asection
+               ? sections.str_offsets.asection->id
+               : 0));
   make_cleanup (xfree, virtual_dwo_name);
   /* Can we use an existing virtual DWO file?  */
   dwo_file_slot = lookup_dwo_file_slot (virtual_dwo_name);
@@ -8981,7 +8982,7 @@ open_and_init_dwp_file (const char *comp
   bfd *dbfd;
   struct cleanup *cleanups;

-  xasprintf (&dwp_name, "%s.dwp", dwarf2_per_objfile->objfile->name);
+  dwp_name = xstrprintf ("%s.dwp", dwarf2_per_objfile->objfile->name);
   cleanups = make_cleanup (xfree, dwp_name);

   dbfd = open_dwop_file (dwp_name, comp_dir, 1);

Re: [RFA] Fix New ARI warning Tue Nov 6 01:58:48 UTC 2012

[Pedro Alves]

On 11/06/2012 11:08 AM, Pierre Muller wrote:

>> 281a282,283
>>> gdb/dwarf2read.c:8612: regression: xasprintf: Do not use xasprintf(),
>> instead use xstrprintf
>> gdb/dwarf2read.c:8612:  xasprintf (&virtual_dwo_name, ',
>>> gdb/dwarf2read.c:8984: regression: xasprintf: Do not use xasprintf(),
>> instead use xstrprintf
>> gdb/dwarf2read.c:8984:  xasprintf (&dwp_name, ', dwarf2_per_objfile-
>>> objfile->name);
> 
> Doug, 
> 
> could you check the patch below
> which removes the new ARI warnings about use of 
> xasprintf function in the commit
> 
> http://sourceware.org/ml/gdb-cvs/2012-11/msg00027.html

Déjà vu :-)

http://sourceware.org/ml/gdb-patches/2009-03/msg00576.html

This removes the last use of xasprintf in the tree; we should delete
it to avoid new uses creeping in.

-- 
Pedro Alves

PING: [RFA] Fix New ARI warning Tue Nov 6 01:58:48 UTC 2012

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : mardi 6 novembre 2012 20:02
> À : Pierre Muller
> Cc : devans@sourceware.org; gdb-patches@sourceware.org
> Objet : Re: [RFA] Fix New ARI warning Tue Nov 6 01:58:48 UTC 2012
> 
> On 11/06/2012 11:08 AM, Pierre Muller wrote:
> 
> >> 281a282,283
> >>> gdb/dwarf2read.c:8612: regression: xasprintf: Do not use xasprintf(),
> >> instead use xstrprintf
> >> gdb/dwarf2read.c:8612:  xasprintf (&virtual_dwo_name, ',
> >>> gdb/dwarf2read.c:8984: regression: xasprintf: Do not use xasprintf(),
> >> instead use xstrprintf
> >> gdb/dwarf2read.c:8984:  xasprintf (&dwp_name, ', dwarf2_per_objfile-
> >>> objfile->name);
> >
> > Doug,
> >
> > could you check the patch below
> > which removes the new ARI warnings about use of
> > xasprintf function in the commit
> >
> > http://sourceware.org/ml/gdb-cvs/2012-11/msg00027.html

  Doug didn't reply yet to this RFA...
 
> Déjà vu :-)
> 
> http://sourceware.org/ml/gdb-patches/2009-03/msg00576.html
> 
> This removes the last use of xasprintf in the tree; we should delete
> it to avoid new uses creeping in.

  There is probably a list of other similar functions
that we should obsolete.
  The main question is, should we do it brutally
or softly.

  1-- Brutal method:
Remove function from both header and C sources
as well as from ARI script.


  2 -- Soft method:
Start by simply removing function from header.
Change function to static.
Wait for one release and suppress completely.

  Method 1 is simpler, but
not adapted to some functions that are used internally.
  Method 2 is gentler to outsiders using gdb sources for
external projects, like the inclusion of libgdb.a into
the Free Pascal IDE, which I basically try to maintain...
Yes, I know I should be using mi, but I never got to 
switch :(

Pierre Muller 
as ARI maintainer

Re: PING: [RFA] Fix New ARI warning Tue Nov 6 01:58:48 UTC 2012

[Joel Brobecker]

>   There is probably a list of other similar functions that we should
>   obsolete.

Can you tell us which functions you have in mind?

>   1-- Brutal method:
> Remove function from both header and C sources
> as well as from ARI script.

I'd go with this approach.

-- 
Joel

RE: PING: [RFA] Fix New ARI warning Tue Nov 6 01:58:48 UTC 2012

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : lundi 12 novembre 2012 16:51
> À : Pierre Muller
> Cc : 'Pedro Alves'; devans@sourceware.org; gdb-patches@sourceware.org
> Objet : Re: PING: [RFA] Fix New ARI warning Tue Nov 6 01:58:48 UTC 2012
> 
> >   There is probably a list of other similar functions that we should
> >   obsolete.
> 
> Can you tell us which functions you have in mind?

  
 It depends if we only consider GDB specific functions
or also functions which are part of some system headers.

For the second category, a possible solution would be to
add or override the function as a macro that would generate an error
at compilation time,
but I am not sure this is easy to do in a way that would 
work for all systems...

  We could forbid by this scheme
function like bcmp/bcopy, abort,
even forbidden constants as true or false
See the list of 'Fixed' items at the bottom of the ARI web page.
 
> >   1-- Brutal method:
> > Remove function from both header and C sources
> > as well as from ARI script.
> 
> I'd go with this approach.

  As long as we reach an agreement on the method...

Pierre 

Re: PING: [RFA] Fix New ARI warning Tue Nov 6 01:58:48 UTC 2012

[Tom Tromey]

>> 1-- Brutal method:
>> Remove function from both header and C sources
>> as well as from ARI script.

Joel> I'd go with this approach.

Me too.  It is better for gdb if the source tree makes sense on its own
terms.  Projects using gdb in "funny" ways can either adapt or copy the
deleted utility functions.

Tom

Re: PING: [RFA] Fix New ARI warning Tue Nov 6 01:58:48 UTC 2012

[Tom Tromey]

>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr> writes:

Pierre> For the second category, a possible solution would be to
Pierre> add or override the function as a macro that would generate an error
Pierre> at compilation time,
Pierre> but I am not sure this is easy to do in a way that would 
Pierre> work for all systems...

IMO it is ok if it doesn't work universally as long as it works for a
large enough subset of gdb developers.  This will ensure that most
breakages are caught early.

GCC uses the poison pragma for this.  It seems to work well enough.

Tom

Re: PING: [RFA] Fix New ARI warning Tue Nov 6 01:58:48 UTC 2012

[Joel Brobecker]

> IMO it is ok if it doesn't work universally as long as it works for a
> large enough subset of gdb developers.  This will ensure that most
> breakages are caught early.
> 
> GCC uses the poison pragma for this.  It seems to work well enough.

Agreed :)

-- 
Joel

Re: PING: [RFA] Fix New ARI warning Tue Nov 6 01:58:48 UTC 2012

[Tom Tromey]

>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr> writes:

>> > could you check the patch below
>> > which removes the new ARI warnings about use of
>> > xasprintf function in the commit
>> >
>> > http://sourceware.org/ml/gdb-cvs/2012-11/msg00027.html

Pierre>   Doug didn't reply yet to this RFA...
 
The patch is ok.

Tom

RE: PING: [RFA] Fix New ARI warning Tue Nov 6 01:58:48 UTC 2012

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Tom Tromey
> Envoyé : mercredi 14 novembre 2012 17:46
> À : Pierre Muller
> Cc : 'Pedro Alves'; devans@sourceware.org; gdb-patches@sourceware.org
> Objet : Re: PING: [RFA] Fix New ARI warning Tue Nov 6 01:58:48 UTC 2012
> 
> >>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr>
writes:
> 
> >> > could you check the patch below
> >> > which removes the new ARI warnings about use of
> >> > xasprintf function in the commit
> >> >
> >> > http://sourceware.org/ml/gdb-cvs/2012-11/msg00027.html
> 
> Pierre>   Doug didn't reply yet to this RFA...
> 
> The patch is ok.

Thank for the approval,
I committed the patch.
 
> Tom

While doing so, I noticed that I forgot to
change the date of the ChangeLog entry concerning the MAINTAINER file.
I just changed it to today also...

Pierre Muller
as ARI maintainer


Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.14829
diff -u -p -r1.14829 ChangeLog
--- ChangeLog   14 Nov 2012 15:18:02 -0000      1.14829
+++ ChangeLog   15 Nov 2012 08:12:12 -0000
@@ -1,4 +1,11 @@
-2012-11-05  Pierre Muller  <muller@sourceware.org>
+2012-11-15  Pierre Muller  <muller@sourceware.org>
+
+       ARI xasprintf rule fixes.
+       * dwarf2read.c (create_dwo_in_dwp): Use xstrprintf function
+       instead of xasprintf.
+       (open_and_init_dwp_file): Ditto.
+
+2012-11-15  Pierre Muller  <muller@sourceware.org>

        * MAINTAINERS (Responsible Maintainers/misc): Add myself
        as responsible of contrib/ari directory.
Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.711
diff -u -p -r1.711 dwarf2read.c
--- dwarf2read.c        12 Nov 2012 17:14:54 -0000      1.711
+++ dwarf2read.c        15 Nov 2012 08:12:14 -0000
@@ -8610,13 +8610,14 @@ create_dwo_in_dwp (struct dwp_file *dwp_
      (fewer struct dwo_file objects to allocated).  Remember that for
really
      large apps there can be on the order of 8K CUs and 200K TUs, or more.
*/

-  xasprintf (&virtual_dwo_name, "virtual-dwo/%d-%d-%d-%d",
-            sections.abbrev.asection ? sections.abbrev.asection->id : 0,
-            sections.line.asection ? sections.line.asection->id : 0,
-            sections.loc.asection ? sections.loc.asection->id : 0,
-            (sections.str_offsets.asection
-             ? sections.str_offsets.asection->id
-             : 0));
+  virtual_dwo_name =
+    xstrprintf ("virtual-dwo/%d-%d-%d-%d",
+               sections.abbrev.asection ? sections.abbrev.asection->id : 0,
+               sections.line.asection ? sections.line.asection->id : 0,
+               sections.loc.asection ? sections.loc.asection->id : 0,
+               (sections.str_offsets.asection
+               ? sections.str_offsets.asection->id
+               : 0));
   make_cleanup (xfree, virtual_dwo_name);
   /* Can we use an existing virtual DWO file?  */
   dwo_file_slot = lookup_dwo_file_slot (virtual_dwo_name);
@@ -8982,7 +8983,7 @@ open_and_init_dwp_file (const char *comp
   bfd *dbfd;
   struct cleanup *cleanups;

-  xasprintf (&dwp_name, "%s.dwp", dwarf2_per_objfile->objfile->name);
+  dwp_name = xstrprintf ("%s.dwp", dwarf2_per_objfile->objfile->name);
   cleanups = make_cleanup (xfree, dwp_name);

   dbfd = open_dwop_file (dwp_name, comp_dir, 1);

RE: PING: [RFA] Fix New ARI warning Tue Nov 6 01:58:48 UTC 2012

[Pierre Muller]

> > The patch is ok.
> 
> Thank for the approval,
> I committed the patch.
> 
> > Tom
> 
> While doing so, I noticed that I forgot to
> change the date of the ChangeLog entry concerning the MAINTAINER file.
> I just changed it to today also...


  Sorry, this is wrong, my commit was 
rejected as my ChangeLog was not uptodate...
But I sent the previous email before noticing the 
rejection of the commit...

As other commits were inserted in the mean time,
I finally did not update the date entry for the MAINTAINERS patch.


  Could someone remind me what the rule about ChangeLog entry date is again?

  It seems that my entry is the only one that makes
a decrease of date bigger than one in the whole ChangeLog of 2012...
But I vaguely remember that I was once told that 
the date should only be updated if there were revisions
in the RFA...
  On the other hand, I also usually change the date to 'today'
before I do a commit, but forgot that for the MAINTAINERS patch.

  Could I change the date of that erntry?

Pierre Muller

Re: PING: [RFA] Fix New ARI warning Tue Nov 6 01:58:48 UTC 2012

[Pedro Alves]

On 15-11-2012 08:31, Pierre Muller wrote:

>   Sorry, this is wrong, my commit was 

It's no big deal.

> rejected as my ChangeLog was not uptodate...
> But I sent the previous email before noticing the 
> rejection of the commit...
> 

> As other commits were inserted in the mean time,
> I finally did not update the date entry for the MAINTAINERS patch.

>   Could someone remind me what the rule about ChangeLog entry date is again?

Date is date of commit.

>   It seems that my entry is the only one that makes
> a decrease of date bigger than one in the whole ChangeLog of 2012...
> But I vaguely remember that I was once told that 
> the date should only be updated if there were revisions
> in the RFA...

Maybe that was another project.  In most GNU projects, the date of
the ChangeLog entry is the commit date.

>   On the other hand, I also usually change the date to 'today'
> before I do a commit, but forgot that for the MAINTAINERS patch.
> 
>   Could I change the date of that erntry?

Please do, and sort it to commit order, if necessary.

-- 
Pedro Alves