News for hackers. Stuff that matters.
Re:Simple Dynamic Strings library for C by unwind on Thursday February 06, 2014 @10:44AM (#7190893)
Nice, thanks for contributing (even more).

I haven't read much of the code at all, but a minor suggestion would be to change:

    struct sdshdr *sh = (void*)(s-(sizeof(struct sdshdr)));
to

    struct sdshdr *sh = (void*)(s-sizeof *sh);
since the entire idea is that the pointer on the left-hand side is to the type whose size should be subtracted, I think it's better not to repeat the type but to "lock it" to the pointer instead. This also (of course) means we can drop the parenthesis with sizeof, since those are only needed when its argument is a type name.
Re:Simple Dynamic Strings library for C by antirez on Thursday February 06, 2014 @10:48AM (#7190910)
Thanks unwind, this is much better indeed! Fixing.

EDIT: Fixed here: https://github.com/antirez/sds/commit/c636fc6cd25e455a75dca2...

Seems everything is still working but I need definitely more unit tests... Note everything is covered right now.

Re:Simple Dynamic Strings library for C by gandalf013 on Thursday February 06, 2014 @02:18PM (#7192329)
A more correct version might be:

    #include <stddef.h>

    int buf_offset = (int)offsetof(struct sdshdr, buf);
    struct sdshdr *sh = s - buf_offset;
This is because the compiler might insert padding between your struct elements and the flexible array member. In your case, you're using only int types in the header, so padding shouldn't be an issue on most architectures, but consider the following:

    struct header {
        int i;
        char c;
        char data[];
    };
sizeof(struct header) on my machine is 8, but "data" starts at an offset of 5 from the beginning of the struct. So, to go from "data" pointer to the pointer representing the beginning of the struct, you will need to subtract 5, not 8. Here is a test program:

    #include <stdio.h>
    #include <stddef.h>

    struct header {
        int i;
        char c;
        char data[];
    };

    int main(void)
    {
        struct header h = {0};
        printf("offsetof %zu\n", offsetof(struct header, data));
        printf("sizeof %zu\n", sizeof h);
        printf("start %p, data start %p, delta %d\n",
                (void *)&h, (void *)(h.data), (int)((char *)h.data - (char *)&h));

        return 0;
    }
http://std.dkuug.dk/JTC1/SC22/WG14/www/docs/n983.htm is relevant for this case as well.
Re:Simple Dynamic Strings library for C by deletes on Thursday February 06, 2014 @03:45PM (#7192817)
Thank you for that.

That surprised me. I thought( and read somewhere ) that the flexible array member comes right after the entire struct, which includes possible padding.

OP got away with it since he always allocates the size of the entire struct, which is 8, plus the string size. And since char doesn't have any alignment requirements it doesn't matter, because he always gets back to correct offset. So data member is basically not used, and if you check the code you will see that it actually in never used!

Re:Simple Dynamic Strings library for C by gandalf013 on Thursday February 06, 2014 @06:21PM (#7193622)
The standard actually specifies (or specified?) that the flexible array member must come after all the members, including the padding. But, from my reading, that wasn't their intent, and is certainly not how compilers implement it. The link in my original post is an acknowledgement from the committee about this and that the standard needs to be updated.

I think the main reason the OP got away with it is because in his structure, "sizeof(struct sdshdr)" is equal to the offset of "buf" in the struct. This is not necessarily true.

In particular, see the latest C standard draft (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf), section 6.7.2.1 (Structure and union specifiers), paragraph 18 and 20-21. An example in the standard uses this code:

    struct s { int n; double d[]; };
and says that "but it is possible that"

    sizeof (struct s) >= offsetof(struct s, d) + sizeof (double)
Re:Simple Dynamic Strings library for C by deletes on Thursday February 06, 2014 @06:37PM (#7193704)
Yeah I was wrong on that, he must have sizes 8 both. My compiler gives 5 and 8.

Coincidentally I just read that specific paragraph earlier today for totally different reasons. :)

Will have to recheck some code.

Re:Simple Dynamic Strings library for C by unwind on Friday February 07, 2014 @03:42AM (#7195345)
Very true of course, I guess I assumed that antirez had thought of that, should have noticed that there were no packing enhchantmens on the struct. I love offsetof() for code like this.
Re:Simple Dynamic Strings library for C by simias on Thursday February 06, 2014 @11:04AM (#7190996)
Heh, you're the first person I meet who advocates dropping the parens around the sizeof argument in any situation. That does look terribly alien to me.
Re:Simple Dynamic Strings library for C by breadbox on Thursday February 06, 2014 @12:33PM (#7191594)
Really? I always do. If you don't, it makes the variable look like a type. It's the same reason why I dislike extra parens around the value in the return statement. Don't make it look like something it isn't.
Re:Simple Dynamic Strings library for C by adestefan on Thursday February 06, 2014 @12:39PM (#7191640)
sizeof is a unary operator and not a function. Only types need to be enclosed in parens. Most people don't know this.
Re:Simple Dynamic Strings library for C by unwind on Friday February 07, 2014 @03:45AM (#7195354)
Exactly! For some reason this anti-pattern is very thoroughly entrenched among C programmers and it's so very frustrating.

My other favorite is casting the return value of malloc(), something you see a great deal of and that I always oppose. See http://stackoverflow.com/questions/605845/do-i-cast-the-resu... for my best arguments.

Re:Simple Dynamic Strings library for C by Narishma on Friday February 07, 2014 @10:54AM (#7196800)
Casting the return value of malloc is necessary in C++. I think that's why it's so widespread, so you can compile your C program with a C++ compiler.