Re: [master] Introduces CHECK_ASPRINTF macro that checks asprintfs return value and terminates program in OOM scenarios.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



-----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

[Index of Archives]     [Kickstart]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]
  Powered by Linux