[SCM] Samba Shared Repository - branch master updated

Jeremy Allison jra at samba.org
Wed Feb 19 18:13:41 MST 2014


On Wed, Feb 19, 2014 at 08:20:04PM +0100, Andreas Schneider wrote:
> The branch, master has been updated

Andreas and David,

>        via  b8844fc clitar.c: check all allocations return value

I *HATE* *HATE* *HATE* the idiom used here.

Calling smb_panic on an allocation fail in the
new tar code is manifestly the *WRONG* thing to
do.

+/* helper macro to die in case of NULL pointer */
+#define PANIC_IF_NULL(x) \
+    _panic_if_null(x, __FILE__ ":" STR2(__LINE__) " (" #x ") == NULL\n")
+
+/* prototype to silent gcc warning */
+static inline void* _panic_if_null(void *p, const char *expr);
+static inline void* _panic_if_null(void *p, const char *expr)
+{
+    if (!p) {
+        smb_panic(expr);
+    }
+    return p;
+}
+

Then later..

+    TALLOC_CTX *ctx = PANIC_IF_NULL(talloc_new(NULL));

and:

+    fname = PANIC_IF_NULL(talloc_asprintf(ctx,
+                                          "%s%s",
+                                          client_get_cur_dir(),
+                                          buf));
     if (!fname) {
         err = 1;
         goto out;

The above even left the (correct) handling of
the NULL return alone !

Remember on common setups this will call a
panic action script which will leave the
client hanging on a sleep 9999999 call and
leave the caller with a hung command and no
indication of what failed.

Can you fix this please to do proper error
checking of a NULL return and to bail out
of the tar command with a correct error indication
that the command failed ?

This really needs fixing properly before this code
can go anywhere near a production branch.

Jeremy.


More information about the samba-cvs mailing list