Snaipe / libcsptr

Smart pointers for the (GNU) C programming language
https://snai.pe/c/c-smart-pointers/
MIT License
1.57k stars 143 forks source link

sentinel uneeded in variadic macro pattern #8

Closed zschoenstadt closed 9 years ago

zschoenstadt commented 9 years ago

The use of the sentinel in the variadic macro pattern is not necessary. The following is valid C and compiles using both gcc-4.4.7 and clang-3.4.2.

edit: Clang also produces no compiler warnings at -Wextra. I understand the desire to maintain ISO compatibility, however, this was not apparent as other GCC/Clang extensions are used, such as the named variadic macro.

edit2: missing context from blog post:

Because the cleanup attribute is a compile time feature, I was going to play around with this on an embedded system like an AVR chip or similar target, where every single bit of RAM counts (most of my stock of AVR chips are 128-512 bytes of ram).

#include <stdlib.h>
#include <stdio.h>
#include <string.h>

#define valloc( T, ...)                \
    ({                                  \
        struct _tmp {                   \
            __typeof__(T) value;        \
        } args = {                      \
            __VA_ARGS__                        \
        };                              \
        void * new = malloc(sizeof(T));  \
        if ( new != NULL)               \
            memcpy( new, &args.value, sizeof(T)); \
        new;                        \
    })

struct vfunc_params {
    int a;
};

void vfunc_real( struct vfunc_params *args) {
    printf("vfunc_param->a = %#x\n", args->a);
}

#define vfunc(...) \
    vfunc_real( &(struct vfunc_params) { __VA_ARGS__ })

int main( int argc, char * argv[])
{
    int * x = 0;

    x = valloc( int );
    printf("valloc returned -> %#x\n", *x);
    free(x);

    x = valloc( int, 0xBAD0BEEF);
    printf("valloc returned -> %#x\n", *x);
    free(x);

    vfunc();
    vfunc(42);

    return 0;
}
Snaipe commented 9 years ago

While this library uses GNU extensions (so non-standard C) and hence is unlikely to have any projects compiling with -pedantic, the sentinel is most surely still needed for those compiling with -Wmissing-field-initializers and -Werror, and nobody will mourn the loss of a padded integer worth of stack size.

Snaipe commented 9 years ago

Since the use case is a bit unusual, I won't change the interface as-is but make it available with an #ifdef'd alternative. Usage would be in the lines of:

#define CSPTR_NO_SENTINEL
#include <csptr/smart_ptr.h>

I could also make it change with ./configure --without-sentinel and the include clause would be unchanged.

Does that sound good ?

Edit: added ./configure alternative

zschoenstadt commented 9 years ago

You do not need to change the interface on my account, since you are right about it making the code ugly. Your point about the ISO C standard is valid, and quite respectable. I can fork if need be.

If you decide to make the change anyway, yes that looks like a great compromise.

zschoenstadt commented 9 years ago

One could use -DCSPTR_NO_SENTINEL on the command line if there was a desire not to define it in the source before the include.

Snaipe commented 9 years ago

One could use -DCSPTR_NO_SENTINEL on the command line if there was a desire not to define it in the source before the include.

After a bit of thinking, this would not work in this case (as well as the #define alternative), as the definition of the arguments structure would differ between the library binary and the user program. I will have to pass it as a switch for ./configure.

Snaipe commented 9 years ago

Fixed in 2.0.3.