swrite.c

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

#define BUFFER_SIZE 5

struct reqdata
{
        char *data;
        size_t len;
        size_t pos;
};

static size_t swrite(char *ptr, size_t size, size_t nmemb,
                     struct reqdata *stream)
{
        if (!stream) return 0;

        size_t cpylen = size * nmemb;
        size_t newlen = stream->len + cpylen + sizeof(char);
        char *data = realloc(stream->data, newlen);

        if (!data) return 0;

        memcpy(data + stream->len, ptr, cpylen);
        data[stream->len + cpylen] = '\0';
        stream->data = data;
        stream->len += cpylen;
        return cpylen;
}

static size_t sread(char *ptr, size_t size, size_t nmemb,
                    struct reqdata *stream)
{
        if (!stream) return 0;

        size_t maxlen = size * nmemb;
        size_t curlen = stream->len - stream->pos;
        size_t cpylen = curlen < maxlen ? curlen : maxlen;
        memcpy(ptr, stream->data + stream->pos, cpylen);
        stream->pos += cpylen;
        return cpylen;
}

int main(int argc, char **argv)
{
        char *s1 = "abcdefg"; //"foofoofoofoofoofoofoo";

        struct reqdata *data1 = malloc(sizeof(struct reqdata));
        data1->data = malloc(sizeof(char));;
        data1->data[0] = '\0';
        data1->len = 0;

        swrite(s1, sizeof(char), BUFFER_SIZE, data1);
        swrite(s1 + BUFFER_SIZE, sizeof(char), BUFFER_SIZE, data1);

        printf("Result: %s\n", data1->data);
        
        free(data1->data);
        free(data1);

        struct reqdata *data2 = malloc(sizeof(struct reqdata));
        data2->data = s1;
        data2->len = strlen(s1);
        data2->pos = 0;

        char *s3 = malloc(sizeof(char) * (BUFFER_SIZE + 1));
        size_t read;

        read = sread(s3, sizeof(char), BUFFER_SIZE, data2);
        s3[read] = '\0';
        printf("Result: %s\n", s3);

        read = sread(s3, sizeof(char), BUFFER_SIZE, data2);
        s3[read] = '\0';
        printf("Result: %s\n", s3);

        free(s3);
        free(data2);

        return 0;
}

==22257== Memcheck, a memory error detector
==22257== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==22257== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==22257== Command: ./main
==22257== 
Result: foofoofoof
Result: foofo
Result: ofoof
==22257== 
==22257== HEAP SUMMARY:
==22257==     in use at exit: 0 bytes in 0 blocks
==22257==   total heap usage: 6 allocs, 6 frees, 72 bytes allocated
==22257== 
==22257== All heap blocks were freed -- no leaks are possible
==22257== 
==22257== For counts of detected and suppressed errors, rerun with: -v
==22257== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)

Here's a (very) basic Makefile for this:

CFLAGS = -std=c99
LDFLAGS = -L/usr/local/lib -lcurl

all: swrite

clean:
    rm -f *.o swrite

.PHONY: clean all

# Make sure that the clean rm line has an actual tab, and not 8 spaces (in vi, ^v^i will do this).

thanks!

• You should be aware of style(9) when writing C code. Some of it is funky to see outside kernel-land, but some of it is good advice.
• Convention for write functions is to return an ssize_t aka "signed size_t". The signedness allows you to return -1 when an error occurs. See for example pwrite(2).
• When dynamically growing buffers, it's pretty common to double the size each time, to avoid frequent reallocations.
• If realloc fails, it will not free the old allocation. reallocf(3) is a FreeBSD extension also present in some other BSDs that does free the old allocation for you. If that is unavailable, you'd need to do something like:

char *buffer = realloc(stream->data, new_len);
if (!buffer) {
    free(stream->data);
    return -1;
}
stream->data = buffer;

• For general purpose data, void * is generally preferable (you get automatic coercion to any other C pointer type on assignment without casts), but you can't do pointer arithmetic with void* (void has no intrinsic size), so char * comes in handy for that. GNU C has an extension that treats sizeof(void) as sizeof(char); it might still warn you that it's leaning on that, though.
• It's not entirely clear based on your parameter names what's getting copied where; srcbuf and dstbuf are conventions you might see to distinguish that. It's also fairly common to sort by-ref params to the end of the function signature, though the standard library is not at all consistent about this.
• I find it better to use sizeof with the variable rather than its type. This leaves one less place to patch things up if you change the variable type later. So rather than struct reqdata *data1 = malloc(sizeof(struct reqdata));, you could write, struct reqdata *data1 = malloc(sizeof(*data1));. This can be especially useful when the declaration of the pointer is far from the malloc, to the point where it's no longer immediately clear the type name passed to sizeof is still that of the pointed to data.
• For variable-length data with a size header in a struct, it's more idiomatic to use a flexible array member:

struct reqdata
{
    size_t len;
    size_t pos;
    char data[];
};

The size of the structure is like that last member wasn't there, but you can reference and index into it; you malloc it like malloc(sizeof(*data1) + sizeof(char)*len), i.e., accounting for both the structure size and the desired array size independently.

This became legit syntax with C99. An earlier version of was supported as a GNU extension with slightly different syntax; the final member was then written as char data[0] IIRC.

• You'd likely be better served by an enum tag or const size_t than a #define. Also, to arm you for macro battle: Macro Pitfalls.

Jeremy's left some solid advice for Jeremy :D

This hits a lot of the notes I was making earlier, but until about 5 minutes ago, I was on a 3G connection and writing long replies was a real pain (I had a lot longer writeup than what we discussed via PM, but Github lost it because "connection timed out"). Now I can address a lot of the things jws discussed (which are all solid points), and give my take on some of them (where and why I do things a little different).

For reference, I follow the OpenBSD style(9); I even have a hardcopy at my desk.

I know we talked about memory management, but you really have to check what each function does in terms of allocating and freeing things. realloc is one example where this will get you. I'd urge you not to rely on GNU extensions, unless you are writing something Linux-specific. It's a tremendous pain for people porting to OS X or one of the BSDs.

Using sizeof(var) is one of the first things I learned writing embedded code. Particularly if you change the struct type that's being used (for example, transitioning to a sockaddr_in6 from a sockaddr_in. I tend to use a struct similar to what jws recommended but I use a char *data. This is probably due to my ANSI C / BSD background, but know you'll probably see that too.

The convention I see most often for the source and destination buffers is src and dst (see, for example, memcpy(3)).

Defines are sometimes useful internally, but you really want to use consts because this will greatly help you out when debugging. Macros (i.e. #defines) won't show up in gdb(1).

On a lot of your calls to sread and swrite, you're not checking for errors. This will bite you in the ass. With C, the more aggressive you are in checking errors early, the easier your life debugging will be.

Really glad you're using valgrind with this. There are a lot of memory leaks that could be obviated if people used valgrind more often. Some other tools to look at are rats(1) and splint(1) -- static checkers.

As a heads up, it's perhaps outside the scope of this, but you can unit test in C with a couple libraries. The one I use is CUnit, and it's in pretty much all the package managers. I've never been on a *nix system that doesn't have it, and it's present in Homebrew.

Finally, writing Makefiles for everything is pretty handy but it gets old fast. I'd recommend looking into the autotools. They're kind of a pain when not done right, and they are a GNU tool (obligatory to point out as a BSD guy ;)), but they make the whole process very easy. That and it's easy to write a target for unit testing. Along with that, learning how to write man pages and/or texinfo will be a boon with C.

One of my example projects you can look at is libsmq. It's not entirely where I want it, but it's got all the parts you'll want to learn to write effective C projects: autotools, man pages, texinfo documentation, and unit tests in addition to the code. You also might look into 21st Century C.

@jeremy-w

i lined up the bullets.

• i think the biggest things i did wrong are: i used 8 spaces instead of 1 tab, struct open paren is on a new line, and i didn't put the function's return type on its own line.
• that's good to know. in this case i am restricted by libcurl requiring size_t.
• makes sense to me.
• after knowing this, i reread the (bsd) manual and it doesn't seem to be explicit. it just says the data is "unchanged" which (now obviously) means not free'd.
• in this case, swrite and sread are intended to be callbacks for libcurl. if libcurl expects void *, wouldn't using char * in the implementation narrow the type and be acceptable? (libcurl also allows you to provide the object being passed in.)
• again, libcurl is dictating parameters here. i just copied the names, and they mention that they copied the names from fread and fwrite. i agree that they're not entirely clear, though.
• hmm, for some reason i thought this would just take the size of a pointer, not the data type.
• this is interesting. does this mean it only requires one call to free? even if data is realloc'd?
• honestly, i don't know why i didn't use a const size_t here. i was probably just caught up in using #define.

thanks for taking the time to provide feedback!

@kisom

i just want to comment in a few things real quick. you and jeremy have given me a lot of stuff to check out and comment further on. (and i really need to get some sleep!) style(9) looks pretty good. it seems like i follow most of it already. i know i am very lacking in error checking in this program. that's the next thing i plan on beefing up before i move this code into my larger project.

thanks so much. i really appreciate it!

void* is a "I won't mess with it and have no idea what it is, so it has to be a pointer so I can at least hold it and pass it back sanely."

The OS X realloc manpage clearly derives from the FreeBSD version, and I guess it's more careful to point out that pitfall because it tripped them up enough to be worth adding their own alternative function that does what you want to do most of the time when the realloc fails.

GNU extensions are fine on OS X. Clang implements pretty much all of them, plus some of its own. At least they've finished implementing the simpler ones from a decade or two ago; some of the fancier ones regarding threading and such are unavailable. The big obnoxious one that's missing is __thread or whatever for transparent thread-local variables – it's only supported for ELF.