Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

I'm a huge fan of the 'parse, don't validate' idiom, but it feels like a bit of a hurdle to use it in C - in order to really encapsulate and avoid errors, you'd need to use opaque pointers to hidden types, which requires the use of malloc (or an object pool per-type or some other scaffolding, that would get quite repetitive after a while, but I digress).

You basically have to trade performance for correctness, whereas in a language like C++, that's the whole purpose of the constructor, which works for all kinds of memory: auto, static, dynamic, whatever.

In C, to initialize a struct without dynamic memory, you could always do the following:

    struct Name {
        const char *name;
    };

    int parse_name(const char *name, struct Name *ret) {
        if(name) {
            ret->name = name;
            return 1;
        } else {
            return 0;
        }
    }

    //in user code, *hopefully*...
    struct Name myname;
    parse_name("mothfuzz", &myname);
But then anyone could just instantiate an invalid Name without calling the parse_name function and pass it around wherever. This is very close to 'validation' type behaviour. So to get real 'parsing' behaviour, dynamic memory is required, which is off-limits for many of the kinds of projects one would use C for in the first place.

I'm very curious as to how the author resolves this, given that they say they don't use dynamic memory often. Maybe there's something I missed while reading.





You can play tricks if you’re willing to compromise on the ABI:

    typedef struct foo_ foo;
    enum { FOO_SIZE = 64 };
    foo *foo_init(void *p, size_t sz);
    void foo_destroy(foo *p);
    #define FOO_ALLOCA() \
      foo_init(alloca(FOO_SIZE), FOO_SIZE)
Implementation (size checks, etc. elided):

    struct foo_ {
        uint32_t magic;
        uint32_t val;
    };
    
    foo *foo_init(void *p, size_t sz) {
        foo *f = (foo *)p;
        f->magic = 1234;
        f->val = 0;
        return f;
    }
Caller:

    foo *f = FOO_ALLOCA();
    // Can’t see inside
    // APIs validate magic

> But then anyone could just instantiate an invalid Name without calling the parse_name function and pass it around wherever

This is nothing new in C. This problem has always existed by virtue of all struct members being public. Generally, programmers know to search the header file / documentation for constructor functions, instead of doing raw struct instantiation. Don‘t underestimate how good documentation can drive correct programming choices.

C++ is worse in this regard, as constructors don‘t really allow this pattern, since they can‘t return a None / false. The alternative is to throw an exception, which requires a runtime similar to malloc.


In C++ you can do: struct Foo { private: int val = 0; Foo(int newVal) : val(newVal) {} public: static optional<Foo> CreateFoo(int newVal) { if (newVal != SENTINEL_VALUE) { return Foo(newVal); } return {}; } };

    int main(int argc, char* argv[]) {
      if (auto f = CreateFoo(argc)) {
        cout << "Foo made with value " << f.val;
      } else {
        cout << "Foo not made";
      }
    }

In C++ you would have a protected constructor and related friend utility class to do the parsing, returning any error code, and constructing the thing, populating an optional, shared_ptr, whatever… don’t make constructors fallible.

Sometimes you want the struct to be defined in a header so it can be passed and returned by value rather than pointer.

A technique I use is to leverage GCC's `poison` pragma to cause an error if attempting to access the struct's fields directly. I give the fields names that won't collide with anything, use macros to access them within the header and then `#undef` the macros at the end of the header.

Example - an immutable, pass-by-value string which couples the `char*` with the length of the string:

    #ifndef FOO_STRING_H
    #define FOO_STRING_H
    
    #include <stddef.h>
    #include <stdlib.h>
    #include <string.h>
    #include "config.h"
    
    typedef size_t string_length_t;
    #define STRING_LENGTH_MAX CONFIG_STRING_LENGTH_MAX
    
    typedef struct {
        string_length_t _internal_string_length;
        char *_internal_string_chars;
    } string_t;
    
    #define STRING_LENGTH(s) (s._internal_string_length)
    #define STRING_CHARS(s) (s._internal_string_chars)
    
    #pragma GCC poison _internal_string_length _internal_string_chars
    
    constexpr string_t error_string = { 0, nullptr };
    constexpr string_t empty_string = { 0, "" };
    
    inline static string_t string_alloc_from_chars(const char *chars) {
        if (chars == nullptr) return error_string;
        size_t len = strnlen(chars, STRING_LENGTH_MAX);
        if (len == 0) return empty_string;
        if (len < STRING_LENGTH_MAX) {
            char *mem = malloc(len + 1);
            strncpy(mem, chars, len);
            mem[len] = '\0';
            return (string_t){ len, mem };
        } else return error_string;
    }
    
    inline static char * string_to_chars(string_t string) {
        return STRING_CHARS(string);
    }

    inline static string_length_t string_length(string_t string) {
        return STRING_LENGTH(string);
    }

    inline static void string_free(string_t s) {
        free(STRING_CHARS(s));
    }
    
    inline static bool string_is_valid(string_t string) {
        return STRING_CHARS(string) != nullptr
            && strnlen(STRING_CHARS(string), STRING_LENGTH_MAX) == STRING_LENGTH(string)
    }
    

    ...

    
    #undef STRING_LENGTH
    #undef STRING_CHARS
    
    #endif /* FOO_STRING_H */
It just wraps `<string.h>` functions in a way that is slightly less error prone to use, and adds zero cost. We can pass the string everywhere by value rather than needing an opaque pointer. It's equivalent on SYSV (64-bit) to passing them as two separate arguments:

    void foo(string_t str);
    //vs
    void foo(size_t length, char *chars); 
These have the exact same calling convention: length passed in `rdi` and `chars` passed in `rsi`. (Or equivalently, `r0:r1` on other architectures).

The main advantage is that we can also return by value without an "out parameter".

    string_t bar();
    //vs
    size_t bar(char **out_chars);
These DO NOT have the same calling convention. The latter is less efficient because it needs to dereference a pointer to return the out parameter. The former just returns length in `rax` and chars in `rdx` (`r0:r1`).

So returning a fat pointer is actually more efficient than returning a size and passing an out parameter on SYSV! (Though only marginally because in the latter case the pointer will be in cache).

Perhaps it's unfair to say "zero-cost" - it's slightly less than zero - cheaper than the conventional idiom of using an out parameter.

But it only works if the struct is <= 16-bytes and contains only INTEGER types. Any larger and the whole struct gets put on the stack for both arguments and returns. In that case it's probably better to use an opaque pointer.

That aside, when we define the struct in the header we can also `inline` most functions, so that avoids unnecessary branching overhead that we might have when using opaque pointers.

`#pragma GCC poison` is not portable, but it will be ignored wherever it isn't supported, so this won't prevent the code being compiled for other platforms - it just won't get the benefits we get from GCC & SYSV.

The biggest downside to this approach is we can't prevent the library user from using a struct initializer and creating an invalid structure (eg, length and actual string length not matching). It would be nice if there were some similar to trick to prevent using compound initializers with the type, then we could have full encapsulation without resorting to opaque pointers.


> The biggest downside to this approach is we can't prevent the library user from using a struct initializer and creating an invalid structure (eg, length and actual string length not matching). It would be nice if there were some similar to trick to prevent using compound initializers with the type, then we could have full encapsulation without resorting to opaque pointers.

Hmm, I found a solution and it was easier than expected. GCC has `__attribute__((designated_init))` we can stick on the struct which prevents positional initializers and requires the field names to be used (assuming -Werror). Since those names are poisoned, we won't be able to initialize except through functions defined in our library. We can similarly use a macro and #undef it.

Full encapsulation of a struct defined in a header:

    #ifndef FOO_STRING_H
    #define FOO_STRING_H

    #include <stddef.h>
    #include <stdlib.h>
    #include <string.h>
    #if defined __has_include
    # if __has_include("config.h")
    #  include "config.h"
    # endif
    #endif

    typedef size_t string_length_t;
    #ifdef CONFIG_STRING_LENGTH_MAX
    #define STRING_LENGTH_MAX CONFIG_STRING_LENGTH_MAX
    #else
    #define STRING_LENGTH_MAX (1 << 24)
    #endif

    typedef struct __attribute__((designated_init)) {
        const string_length_t _internal_string_length;
        const char *const _internal_string_chars;
    } string_t;

    #define STRING_CREATE(len, ptr) (string_t){ ._internal_string_length = (len), ._internal_string_chars = (ptr) }
    #define STRING_LENGTH(s) (s._internal_string_length)
    #define STRING_CHARS(s) (s._internal_string_chars)
    #pragma GCC poison _internal_string_length _internal_string_chars


    constexpr string_t error_string = STRING_CREATE(0, nullptr);
    constexpr string_t empty_string = STRING_CREATE(0, "");

    inline static string_t string_alloc_from_chars(const char *chars) {
        if (__builtin_expect(chars == nullptr, false)) return error_string;
        size_t len = strnlen(chars, STRING_LENGTH_MAX);
        if (__builtin_expect(len == 0, false)) return empty_string;
        if (__builtin_expect(len < STRING_LENGTH_MAX, true)) {
            char *mem = malloc(len + 1);
            strncpy(mem, chars, len);
            mem[len] = '\0';
            return STRING_CREATE(len, mem);
        } else return error_string;
    }

    inline static const char *string_to_chars(string_t string) {
        return STRING_CHARS(string);
    }

    inline static string_length_t string_length(string_t string) {
        return STRING_LENGTH(string);
    }

    inline static void string_free(string_t s) {
        free((char*)STRING_CHARS(s));
    }

    inline static bool string_is_valid(string_t string) {
        return STRING_CHARS(string) != nullptr;
    }

    // ... other string function

    #undef STRING_LENGTH
    #undef STRING_CHARS
    #undef STRING_CREATE

    #endif /* FOO_STRING_H */
Aside from horrible pointer aliasing tricks, the only way to create a `string_t` is via `string_alloc_from_chars` or other functions defined in the library which return `string_t`.

    #include <stdio.h>
    int main() {
        string_t s = string_alloc_from_chars("Hello World!");
        if (string_is_valid(s)) 
            puts(string_to_chars(s));
        string_free(s);
        return 0;
    }

If you don't want your types to be public, don't put them in the public interface, put them into the implementation.



Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: