* New ARI warning Fri Feb 27 01:54:21 UTC 2015 @ 2015-02-27 1:54 GDB Administrator 2015-02-27 8:24 ` Joel Brobecker 0 siblings, 1 reply; 6+ messages in thread From: GDB Administrator @ 2015-02-27 1:54 UTC (permalink / raw) To: gdb-patches 16a17 > gdb/amd64fbsd-tdep.c:54: code: OP eol: Do not use &&, or || at the end of a line gdb/amd64fbsd-tdep.c:54: if (memcmp (buf, amd64fbsd_sigtramp_code, sizeof amd64fbsd_sigtramp_code) != 242a244,250 > gdb/i386fbsd-tdep.c:108: code: OP eol: Do not use &&, or || at the end of a line gdb/i386fbsd-tdep.c:108:gdb_static_assert (sizeof i386fbsd_sigtramp_start == > gdb/i386fbsd-tdep.c:110: code: OP eol: Do not use &&, or || at the end of a line gdb/i386fbsd-tdep.c:110:gdb_static_assert (sizeof i386fbsd_sigtramp_start == > gdb/i386fbsd-tdep.c:112: code: OP eol: Do not use &&, or || at the end of a line gdb/i386fbsd-tdep.c:112:gdb_static_assert (sizeof i386fbsd_sigtramp_middle == > gdb/i386fbsd-tdep.c:114: code: OP eol: Do not use &&, or || at the end of a line gdb/i386fbsd-tdep.c:114:gdb_static_assert (sizeof i386fbsd_sigtramp_middle == > gdb/i386fbsd-tdep.c:116: code: OP eol: Do not use &&, or || at the end of a line gdb/i386fbsd-tdep.c:116:gdb_static_assert (sizeof i386fbsd_sigtramp_end == > gdb/i386fbsd-tdep.c:118: code: OP eol: Do not use &&, or || at the end of a line gdb/i386fbsd-tdep.c:118:gdb_static_assert (sizeof i386fbsd_sigtramp_end == > gdb/i386fbsd-tdep.c:138: code: OP eol: Do not use &&, or || at the end of a line gdb/i386fbsd-tdep.c:138: if (memcmp (buf, i386fbsd_sigtramp_start, sizeof i386fbsd_sigtramp_start) == ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: New ARI warning Fri Feb 27 01:54:21 UTC 2015 2015-02-27 1:54 New ARI warning Fri Feb 27 01:54:21 UTC 2015 GDB Administrator @ 2015-02-27 8:24 ` Joel Brobecker 2015-02-27 19:01 ` John Baldwin 0 siblings, 1 reply; 6+ messages in thread From: Joel Brobecker @ 2015-02-27 8:24 UTC (permalink / raw) To: John Baldwin; +Cc: gdb-patches, Pedro Alves Hello John, I think you patch caused the following warnings. Could you take a look and fix? Also, while looking at the patch to confirm the source of the warnings, I noticed curly braces on the same line as if conditions. Our coding standard is to have them on the next line. Thus... if (condition) { [...] } ... should be ... if (condition) { [...] } Thank you! On Fri, Feb 27, 2015 at 01:54:22AM +0000, GDB Administrator wrote: > 16a17 > > gdb/amd64fbsd-tdep.c:54: code: OP eol: Do not use &&, or || at the end of a line > gdb/amd64fbsd-tdep.c:54: if (memcmp (buf, amd64fbsd_sigtramp_code, sizeof amd64fbsd_sigtramp_code) != > 242a244,250 > > gdb/i386fbsd-tdep.c:108: code: OP eol: Do not use &&, or || at the end of a line > gdb/i386fbsd-tdep.c:108:gdb_static_assert (sizeof i386fbsd_sigtramp_start == > > gdb/i386fbsd-tdep.c:110: code: OP eol: Do not use &&, or || at the end of a line > gdb/i386fbsd-tdep.c:110:gdb_static_assert (sizeof i386fbsd_sigtramp_start == > > gdb/i386fbsd-tdep.c:112: code: OP eol: Do not use &&, or || at the end of a line > gdb/i386fbsd-tdep.c:112:gdb_static_assert (sizeof i386fbsd_sigtramp_middle == > > gdb/i386fbsd-tdep.c:114: code: OP eol: Do not use &&, or || at the end of a line > gdb/i386fbsd-tdep.c:114:gdb_static_assert (sizeof i386fbsd_sigtramp_middle == > > gdb/i386fbsd-tdep.c:116: code: OP eol: Do not use &&, or || at the end of a line > gdb/i386fbsd-tdep.c:116:gdb_static_assert (sizeof i386fbsd_sigtramp_end == > > gdb/i386fbsd-tdep.c:118: code: OP eol: Do not use &&, or || at the end of a line > gdb/i386fbsd-tdep.c:118:gdb_static_assert (sizeof i386fbsd_sigtramp_end == > > gdb/i386fbsd-tdep.c:138: code: OP eol: Do not use &&, or || at the end of a line > gdb/i386fbsd-tdep.c:138: if (memcmp (buf, i386fbsd_sigtramp_start, sizeof i386fbsd_sigtramp_start) == -- Joel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: New ARI warning Fri Feb 27 01:54:21 UTC 2015 2015-02-27 8:24 ` Joel Brobecker @ 2015-02-27 19:01 ` John Baldwin 2015-03-02 15:02 ` Joel Brobecker 0 siblings, 1 reply; 6+ messages in thread From: John Baldwin @ 2015-02-27 19:01 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches, Pedro Alves On Friday, February 27, 2015 09:24:42 AM Joel Brobecker wrote: > Hello John, > > I think you patch caused the following warnings. Could you take > a look and fix? > > Also, while looking at the patch to confirm the source of the warnings, > I noticed curly braces on the same line as if conditions. Our coding > standard is to have them on the next line. Thus... > > if (condition) { > [...] > } > > ... should be ... > > if (condition) > { > [...] > } > > Thank you! Yes, I've tried to follow that in my other patches (some of which I haven't sent yet), but most of the code I work on follows the other style and old habits die hard. :) Not sure if these sorts of fixes need a ChangeLog entry as well, but does this address these? Style fixes. - Do not leave operators at end-of-line. - Fix block indentation in if-else chain. gdb/ChangeLog: * amd64fbsd-tdep.c (amd64fbsd_sigtramp_p): Style fixes. * i386fbsd-tdep.c (i386fbsd_sigtramp_p): Likewise. --- gdb/amd64fbsd-tdep.c | 4 ++-- gdb/i386fbsd-tdep.c | 64 ++++++++++++++++++++++++++++------------------------ 2 files changed, 37 insertions(+), 31 deletions(-) diff --git a/gdb/amd64fbsd-tdep.c b/gdb/amd64fbsd-tdep.c index e11b0f3..62dcb83 100644 --- a/gdb/amd64fbsd-tdep.c +++ b/gdb/amd64fbsd-tdep.c @@ -51,8 +51,8 @@ amd64fbsd_sigtramp_p (struct frame_info *this_frame) if (!safe_frame_unwind_memory (this_frame, pc, buf, sizeof buf)) return 0; - if (memcmp (buf, amd64fbsd_sigtramp_code, sizeof amd64fbsd_sigtramp_code) != - 0) + if (memcmp (buf, amd64fbsd_sigtramp_code, sizeof amd64fbsd_sigtramp_code) + != 0) return 0; return 1; diff --git a/gdb/i386fbsd-tdep.c b/gdb/i386fbsd-tdep.c index d4516ee..ed41706 100644 --- a/gdb/i386fbsd-tdep.c +++ b/gdb/i386fbsd-tdep.c @@ -105,24 +105,24 @@ static const gdb_byte i386fbsd_osigtramp_end[] = }; /* The three different trampolines are all the same size. */ -gdb_static_assert (sizeof i386fbsd_sigtramp_start == - sizeof i386fbsd_freebsd4_sigtramp_start); -gdb_static_assert (sizeof i386fbsd_sigtramp_start == - sizeof i386fbsd_osigtramp_start); -gdb_static_assert (sizeof i386fbsd_sigtramp_middle == - sizeof i386fbsd_freebsd4_sigtramp_middle); -gdb_static_assert (sizeof i386fbsd_sigtramp_middle == - sizeof i386fbsd_osigtramp_middle); -gdb_static_assert (sizeof i386fbsd_sigtramp_end == - sizeof i386fbsd_freebsd4_sigtramp_end); -gdb_static_assert (sizeof i386fbsd_sigtramp_end == - sizeof i386fbsd_osigtramp_end); +gdb_static_assert (sizeof i386fbsd_sigtramp_start + == sizeof i386fbsd_freebsd4_sigtramp_start); +gdb_static_assert (sizeof i386fbsd_sigtramp_start + == sizeof i386fbsd_osigtramp_start); +gdb_static_assert (sizeof i386fbsd_sigtramp_middle + == sizeof i386fbsd_freebsd4_sigtramp_middle); +gdb_static_assert (sizeof i386fbsd_sigtramp_middle + == sizeof i386fbsd_osigtramp_middle); +gdb_static_assert (sizeof i386fbsd_sigtramp_end + == sizeof i386fbsd_freebsd4_sigtramp_end); +gdb_static_assert (sizeof i386fbsd_sigtramp_end + == sizeof i386fbsd_osigtramp_end); /* We assume that the middle is the largest chunk below. */ -gdb_static_assert (sizeof i386fbsd_sigtramp_middle > - sizeof i386fbsd_sigtramp_start); -gdb_static_assert (sizeof i386fbsd_sigtramp_middle > - sizeof i386fbsd_sigtramp_end); +gdb_static_assert (sizeof i386fbsd_sigtramp_middle + > sizeof i386fbsd_sigtramp_start); +gdb_static_assert (sizeof i386fbsd_sigtramp_middle + > sizeof i386fbsd_sigtramp_end); static int i386fbsd_sigtramp_p (struct frame_info *this_frame) @@ -135,19 +135,25 @@ i386fbsd_sigtramp_p (struct frame_info *this_frame) if (!safe_frame_unwind_memory (this_frame, pc, buf, sizeof i386fbsd_sigtramp_start)) return 0; - if (memcmp (buf, i386fbsd_sigtramp_start, sizeof i386fbsd_sigtramp_start) == - 0) { - middle = i386fbsd_sigtramp_middle; - end = i386fbsd_sigtramp_end; - } else if (memcmp (buf, i386fbsd_freebsd4_sigtramp_start, - sizeof i386fbsd_freebsd4_sigtramp_start) == 0) { - middle = i386fbsd_freebsd4_sigtramp_middle; - end = i386fbsd_freebsd4_sigtramp_end; - } else if (memcmp (buf, i386fbsd_osigtramp_start, - sizeof i386fbsd_osigtramp_start) == 0) { - middle = i386fbsd_osigtramp_middle; - end = i386fbsd_osigtramp_end; - } else + if (memcmp (buf, i386fbsd_sigtramp_start, sizeof i386fbsd_sigtramp_start) + == 0) + { + middle = i386fbsd_sigtramp_middle; + end = i386fbsd_sigtramp_end; + } + else if (memcmp (buf, i386fbsd_freebsd4_sigtramp_start, + sizeof i386fbsd_freebsd4_sigtramp_start) == 0) + { + middle = i386fbsd_freebsd4_sigtramp_middle; + end = i386fbsd_freebsd4_sigtramp_end; + } + else if (memcmp (buf, i386fbsd_osigtramp_start, + sizeof i386fbsd_osigtramp_start) == 0) + { + middle = i386fbsd_osigtramp_middle; + end = i386fbsd_osigtramp_end; + } + else return 0; /* Since the end is shorter than the middle, check for a matching end -- 2.2.1 -- John Baldwin ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: New ARI warning Fri Feb 27 01:54:21 UTC 2015 2015-02-27 19:01 ` John Baldwin @ 2015-03-02 15:02 ` Joel Brobecker 2015-03-12 19:37 ` John Baldwin 0 siblings, 1 reply; 6+ messages in thread From: Joel Brobecker @ 2015-03-02 15:02 UTC (permalink / raw) To: John Baldwin; +Cc: gdb-patches, Pedro Alves > Yes, I've tried to follow that in my other patches (some of which I haven't > sent yet), but most of the code I work on follows the other style and old > habits die hard. :) :-) > Not sure if these sorts of fixes need a ChangeLog entry as well, but does > this address these? Yes, all changes require a ChangeLog entry (I have this wild and impossible dream that, one day, we will get rid of these ChangeLogs. One day...) > gdb/ChangeLog: > > * amd64fbsd-tdep.c (amd64fbsd_sigtramp_p): Style fixes. > * i386fbsd-tdep.c (i386fbsd_sigtramp_p): Likewise. > diff --git a/gdb/i386fbsd-tdep.c b/gdb/i386fbsd-tdep.c > index d4516ee..ed41706 100644 > --- a/gdb/i386fbsd-tdep.c > +++ b/gdb/i386fbsd-tdep.c > @@ -105,24 +105,24 @@ static const gdb_byte i386fbsd_osigtramp_end[] = > }; > > /* The three different trampolines are all the same size. */ > -gdb_static_assert (sizeof i386fbsd_sigtramp_start == > - sizeof i386fbsd_freebsd4_sigtramp_start); > -gdb_static_assert (sizeof i386fbsd_sigtramp_start == > - sizeof i386fbsd_osigtramp_start); > -gdb_static_assert (sizeof i386fbsd_sigtramp_middle == > - sizeof i386fbsd_freebsd4_sigtramp_middle); > -gdb_static_assert (sizeof i386fbsd_sigtramp_middle == > - sizeof i386fbsd_osigtramp_middle); > -gdb_static_assert (sizeof i386fbsd_sigtramp_end == > - sizeof i386fbsd_freebsd4_sigtramp_end); > -gdb_static_assert (sizeof i386fbsd_sigtramp_end == > - sizeof i386fbsd_osigtramp_end); > +gdb_static_assert (sizeof i386fbsd_sigtramp_start > + == sizeof i386fbsd_freebsd4_sigtramp_start); > +gdb_static_assert (sizeof i386fbsd_sigtramp_start > + == sizeof i386fbsd_osigtramp_start); > +gdb_static_assert (sizeof i386fbsd_sigtramp_middle > + == sizeof i386fbsd_freebsd4_sigtramp_middle); > +gdb_static_assert (sizeof i386fbsd_sigtramp_middle > + == sizeof i386fbsd_osigtramp_middle); > +gdb_static_assert (sizeof i386fbsd_sigtramp_end > + == sizeof i386fbsd_freebsd4_sigtramp_end); > +gdb_static_assert (sizeof i386fbsd_sigtramp_end > + == sizeof i386fbsd_osigtramp_end); > > /* We assume that the middle is the largest chunk below. */ > -gdb_static_assert (sizeof i386fbsd_sigtramp_middle > > - sizeof i386fbsd_sigtramp_start); > -gdb_static_assert (sizeof i386fbsd_sigtramp_middle > > - sizeof i386fbsd_sigtramp_end); > +gdb_static_assert (sizeof i386fbsd_sigtramp_middle > + > sizeof i386fbsd_sigtramp_start); > +gdb_static_assert (sizeof i386fbsd_sigtramp_middle > + > sizeof i386fbsd_sigtramp_end); You'll need to mention those in the ChangeLog as well. Since there is no function as the context, I would probably say something like this: * i386fbsd-tdep.c: Fix style in various gdb_static_assert expressions. ? Otherwise, looks good to me. Thanks! -- Joel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: New ARI warning Fri Feb 27 01:54:21 UTC 2015 2015-03-02 15:02 ` Joel Brobecker @ 2015-03-12 19:37 ` John Baldwin 2015-03-13 17:50 ` Pedro Alves 0 siblings, 1 reply; 6+ messages in thread From: John Baldwin @ 2015-03-12 19:37 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches, Pedro Alves On Monday, March 02, 2015 07:01:55 AM Joel Brobecker wrote: > You'll need to mention those in the ChangeLog as well. > Since there is no function as the context, I would probably say > something like this: > > * i386fbsd-tdep.c: Fix style in various gdb_static_assert > expressions. > > ? How about this? gdb/ChangeLog: * amd64fbsd-tdep.c (amd64fbsd_sigtramp_p): Style fixes. * i386fbsd-tdep.c: Fix style in various gdb_static_assert expressions. (i386fbsd_sigtramp_p): Likewise. -- John Baldwin ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: New ARI warning Fri Feb 27 01:54:21 UTC 2015 2015-03-12 19:37 ` John Baldwin @ 2015-03-13 17:50 ` Pedro Alves 0 siblings, 0 replies; 6+ messages in thread From: Pedro Alves @ 2015-03-13 17:50 UTC (permalink / raw) To: John Baldwin, Joel Brobecker; +Cc: gdb-patches On 03/12/2015 07:18 PM, John Baldwin wrote: > gdb/ChangeLog: > > * amd64fbsd-tdep.c (amd64fbsd_sigtramp_p): Style fixes. > * i386fbsd-tdep.c: Fix style in various gdb_static_assert > expressions. > (i386fbsd_sigtramp_p): Likewise. That's great. OK. Please push. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-03-13 17:50 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-02-27 1:54 New ARI warning Fri Feb 27 01:54:21 UTC 2015 GDB Administrator 2015-02-27 8:24 ` Joel Brobecker 2015-02-27 19:01 ` John Baldwin 2015-03-02 15:02 ` Joel Brobecker 2015-03-12 19:37 ` John Baldwin 2015-03-13 17:50 ` Pedro Alves
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).