Hi guys,
I do not really agree with Peter. The macro explodes itself into exactly one line (note the usage of backslashes). So __line__ and __func__ will both be correct. Also I'm pretty sure, that cpp uses line number from the original .c file, not from the preprocessed version of it (all header files are also present in preprocesed version of the .c file, so it would not make sense).
Lowercase naming might be a good idea, but the macro itself seems OK to me.
Martin
----- "David Cantrell" <dcantrell@xxxxxxxxxx> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On Mon, 16 Nov 2009, Peter Jones wrote:
>
> > On 11/13/2009 11:22 AM, Ales Kozumplik wrote:
> >> This is to avoid having to copy-paste the asprintf-log-abort if
> branch all the time.
> >> ---
> >> loader/loader.h | 6 +++
> >> loader/nfsinstall.c | 125
> +++++++++++++--------------------------------------
> >> 2 files changed, 38 insertions(+), 93 deletions(-)
> >>
> >> diff --git a/loader/loader.h b/loader/loader.h
> >> index f942b7e..039c72a 100644
> >> --- a/loader/loader.h
> >> +++ b/loader/loader.h
> >> @@ -185,4 +185,10 @@ struct loaderData_s {
> >> #define LIBPATH
> "/lib:/usr/lib:/usr/X11R6/lib:/usr/kerberos/lib:/mnt/usr/lib:/mnt/sysimage/lib:/mnt/sysimage/usr/lib"
> >> #endif
> >>
> >> +#define CHECKED_ASPRINTF(...)
> \
> >> + if (asprintf( __VA_ARGS__ ) == -1) {
> \
> >> + logMessage(CRITICAL, "%s: %d: %m", __func__, __LINE__);
> \
> >> + abort();
> \
> >> + }
> >> +
> >> #endif
> >
> > So, the big problem with doing this is that it will call the error
> message
> > to be incorrect. If you're going to do this, you actually need to
> do:
> >
> > #define checked_asprintf(...) ({
> \
> > char *_f = __func__; int _l = __LINE__;
> \
> > if (asprintf( __VA_ARGS__ ) == -1) {
> \
> > logMessage(CRITICAL, "%s: %d: %m", _f, _d);
> \
> > abort();
> \
> > }
> \
> > })
> >
> > Or else the line number in the log file will be a line _after_ the
> > checked_asprintf() call.
> >
> > This is why I don't like tricky macros.
>
> My first thought on the macro for using asprintf() was "no", but in an
> effort
> to encourage development, I can put aside my opposition to macros.
> But Peter
> is right here and I don't use macros enough ever for things like this
> to even
> notice the problem. Which leads me to now side with him. If we get
> in to the
> business of having difficult to follow code and confusing macros, it
> makes the
> code base that much more difficult to maintain down the road.
>
> The macro as edited by Peter is the sort of thing someone might want
> to clean
> up later on down the road, not realizing we need that construct to
> ensure
> error reporting is correct.
>
> Also, going this route is trusting the preprocessor for too much
> stuff.
> Trusting cpp only leads to pain and suffering.
>
> - --
> David Cantrell <dcantrell@xxxxxxxxxx>
> Red Hat / Honolulu, HI
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iEYEARECAAYFAksBuHkACgkQ5hsjjIy1VklsEgCg2gs+1d4DZXUFlTFzAUB5grFW
> X+sAnj3S5zU98ZPvHnjO+iMokqW1l/sn
> =bPov
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
