Splitting a cookie string into a 2D array

I am writing possibly my first real C project, and it is almost finished. But as I was coming to a close I noticed something.

This is a simple operation to split a cookie string into a ~2D array. I don't expect you to read all of the code if you don't want to. It works, but that is not my current worry.

#include 
#include 
#include 
#include 
#include 

using namespace std;

typedef struct _http_cookie_t
{
    const char * key;
    const char * val;
    unsigned int key_len;
    unsigned int val_len;
} http_cookie_t;

int cookies_get_length(char * s);
int cookies_contains(http_cookie_t ** cookies, int length, const char * string);
int cookies_create_map(http_cookie_t ** cookies, int length, char * string);
int cookies_separate_string(char ** cookies, int length, char * string);
int cookies_create_single(http_cookie_t * cookie, char * src);

int main() {

    const char * src = "UREF1=F2E2D08C;UREF2=F2E2D08C";

    const int length = cookies_get_length((char *)src);
    http_cookie_t ** cookies = (http_cookie_t **)malloc(length * sizeof(struct http_cookie_t));
    cookies_create_map(cookies, length, (char *)src);

    if (cookies_contains(cookies, length, "UREF")) {
        printf("WAS FOUND");
    } else {
        printf("NOT FOUND");
    }

    return 0;
}

int cookies_contains(http_cookie_t ** cookies, int length, const char * string)
{
    int located = 0;
    int i;

    for (i = 0; i key)) {
            located = 1;
            break;
        }
    }

    return located;
}

int cookies_create_map(http_cookie_t ** cookies, int length, char * string)
{
    char * strings[length];
    cookies_separate_string(strings, length, string);

    int i;
    for (i = 0; ikey = strings[0];
    cookie->val = strings[1];

    cookie->key_len = (int)strlen(cookie->key);
    cookie->val_len = (int)strlen(cookie->val);

    return 1;
}

int cookies_get_length(char * s)
{
    int i;
    for (i=0; s[i]; s[i]==';' ? i++ : *s++);

    return i + 1;
}

My concern is that I

Here are some things that may help your improve your code.

Decide which language you're using

The code looks mostly like C, but won't compile as C because it has a few things that are C++. I'm going to assume you're really intending to write C for this review. If it were C++, I'd suggest a completely different approach.

Remove things that aren't C

Remove #include and using namespace std because they are not C.

Use your typedefs

The code correctly and reasonably defines a typedef for http_cookie_t but then uses lines like this:

cookies[i] = (http_cookie_t *)malloc(sizeof(struct http_cookie_t));

First, that won't (or shouldn't) compile because the struct actually has a leading underscore (_http_cookie_t), and second, you can use the typedef to simplify the code:

cookies[i] = (http_cookie_t *)malloc(sizeof(http_cookie_t));

Use pointers rather than indexing

In C, it's usually more efficient to use pointers rather than indexing. Even if you don't care too much about the performance of the code, this also makes the code more idomatic C. You almost certainly don't want to use both except in very special circumstances. So in cookies_get_length, for instance, the code is currently this:

int cookies_get_length(char * s)
{
    int i;
    for (i=0; s[i]; s[i]==';' ? i++ : *s++);

    return i + 1;
}

This is broken code because of the last part of the for clause. The expression s[i]==';' ? i++ : s++ says that if s[i] is a semicolon, increment i, otherwise increment s. If you feed this code this string ";" it will never terminate. If you feed it a NULL pointer, it will crash. Do you see why? A better way to write the loop would be this:

int cookies_get_count(const char * s)
{
    int cookie_count = 0;
    if (s == NULL)
        return cookie_count;
    for (++cookie_count; *s; ++s)
        if (*s == ';')
            ++cookie_count;
    return cookie_count;
}

Note that the code bails out early if it gets a NULL pointer, that the loop is much simpler and that the name is more descriptive -- it's really a count of cookies, rather than the length. Also, the parameter is declared as const char *s.

Don't cast result of malloc

The malloc call returns a void * and one of the special aspects of C is that such a type does not need to be cast to be converted into another pointer type. So for example, this line:

http_cookie_t ** cookies = (http_cookie_t **)malloc(length * sizeof(http_cookie_t));

could be shortened to this:

http_cookie_t ** cookies = malloc(length * sizeof(http_cookie_t));

However, you should check the return value to assure it isn't NULL because that is an indication that the program has run out of memory and is a serious error.

Eliminate unused variables

In the cookies_separate_string routine, the parameter length is never used and can be eliminated.

Use const where practical

In all of your functions, the last parameter is a pointer to a string. In all cases, that string is unmodified by the function and so can (and should) be declared as const char *.

Consider a more efficient algorithm

Right now, the code makes many passes through the same string, but it's quite possible to do everything in a single pass. Such an algorithm would likely shorten the code considerably.

Don't leak memory

This code calls malloc several places but never free. This means that the routines are leaking memory. It would be much better to get into the habit of using free for each call to malloc and then assuring that you don't leak memory.

StackExchange Code Review Q#73806, answer score: 3