This is the mail archive of the elfutils-devel@sourceware.org mailing list for the elfutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] libdwfl: Correct nested asprintf result check in report_kernel_archive.


On Sat, Dec 21, 2013 at 12:46:19PM -0500, Frank Ch. Eigler wrote:
> > Yeah, I think you are right, I overdid it because I was a little embarrassed
> > I had to count the brackets 6 times before seeing the bug even after it was
> > pointed out to me. Pushed the simplified version:
> 
> Aw heck, at that point, why not
> 
> int res = asprintf (&archive,
>                     ((*release)[0]=='/' ? "%s/debug.a"
>                                         : MODULEDIRFMT "/debug.a"),
>                     *release);

Yeah, that might also be an option.
Nitpick, that would format as:

  int res = asprintf (&archive,
                      ((*release)[0]=='/' ? "%s/debug.a"
                       : MODULEDIRFMT "/debug.a"),
                      *release);

or

  int res = asprintf (&archive,
                      ((*release)[0]=='/'
                       ? "%s/debug.a"
                       : MODULEDIRFMT "/debug.a"),
                      *release);

The ? and : line up with the character after the previous opening bracket.
(Emacs is your friend.)

It is just a matter of taste I guess whether to prefer the two larger
asprintf functions on their own lines or have a smaller tenary if
expression. I have a (very small) preference for the first which IMHO
reads just a tiny bit easier. But that "advantage" would not be there
with your formatting style (it is just that emacs will complain about
it and we don't want to anger emacs now do we?)

Cheers,

Mark

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]