Custom strcat() with different arguments

Question

I am teaching myself C, and feel like I am just starting to get the hang of pointers and arrays (I come from Python, where everything is magic). I'm looking for reviews, especially if I'm doing anything wrong. In this code I wrote my own strcat() function, though it needs 3 args instead of the standard 2 (I couldn't figure out how to do it with just 2 without overrunning allotted memory).

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

char* cat(char *dest, char *a, char*b)
{   
    int len_a = strlen(a);
    int len_b = strlen(b);
    int i;
    for(i = 0; i < len_a; i++)
    {
        dest[i] = a[i];
    }
    puts("");
    int j;
    for(j = 0;j < len_b; i++, j++)
    {
        dest[i] = b[j];
    }
    dest[i] = '\0';
    puts("FUNCTION FINISHED");
}

int main()
{
    char strA[] = "I am a small ";
    char strB[] = "cat with whiskers.";
    char strC[strlen(strA) + strlen(strB) + 1];
    printf("length A: %lu, B: %lu, C: %lu\n", strlen(strA), strlen(strB), strlen(strC));
    printf("sizeof A: %lu, B: %lu, C: %lu\n", sizeof(strA), sizeof(strB), sizeof(strC));
    printf("A: '%s'\nB: '%s'\n", strA, strB);
    cat(strC, strA, strB);
    printf("c: '%s'\n", strC);
    printf("length A: %lu, B: %lu, C: %lu\n", strlen(strA), strlen(strB), strlen(strC));


    return 0;
}

Answer 1

char* cat(char *dest, char *a, char*b)

Interface critique: You should have the caller specify a maximum size for the destination buffer, and error out when there is not enough space. The mark of a good C programmer is to create interfaces which make this sort of condition unambiguous, rather than blasting away on the buffer, potentially past the allocation size.

for(i = 0; i < len_a; i++)
{
    dest[i] = a[i];
}

Strictly speaking this is fine, but it is rather un-C-like. I would prefer:

while (*a)
   *dest++ = *a++;

With this you do not need to call strlen, either.

Assuming you added the parameter you should add in my interface critique (let's call the new parameter destsz), you could make sure you don't write more than this size with something like:

int cat(char *dest, size_t destsz, const char *a, const char *b)
{
    while (*a && destsz)
    {
       *dest++ = *a++;
       --destsz;
    }

    while (*b && destsz)
    {
       *dest++ = *b++;
       --destsz;
    }

    if (!destsz)
       return -1; // ran out of buffer; error condition

    *dest = 0;   // we have space, so write NUL
    return 0;
}

Answer 2

Just a few things to add to john.k.doe's answer:

cat's strlen calls

The implementation of strlen is usually something vaguely like:

size_t strlen(const char* str)
{
    size_t len = 0;
    while (str[len] != 0) {
        ++len;
    }
    return len;
}

This has two significant effects:

• Use size_t instead of an int when working with functions that return a size_t. It often doesn't matter, but it's a bad habit. (For example, file sizes often are larger than an int -- though I'm not actually sure if there's a file size function that returns a size_t... :).)
• No need to loop twice

On the second point, what I mean is that you're basically looping twice. You could just use this construct instead:

char* cat(char* dest, const char* a, const char* b)
{
    size_t off = 0;
    size_t idx = 0;
    while (a[off]) {
        dest[off] = a[off];
        ++off;
    }
    idx = 0;
    while (b[idx]) {
        dest[off] = b[idx];
        ++off;
        ++idx;
    }
    return dest;
}

Mark arguments as const when possible

a and b should be marked as const since the values pointed to by the pointer are not changed in the function. Using const is a way to allow the compiler to do certain optimization, but more importantly, it's a signal to other sections of code that "I can pass something to this function, and I know that this function won't modify it."

My rule of thumb is to have all pointers const (at least in my head) and then un-const them only when necessary.

I should probably also mention that there's different placements for const and they mean different things. Statements are usually most logically apparently from right to left when thinking about const.

const char* a;

From right to left this would read, "a is a pointer to a char that is const."

This means that a can be changed, but derefencing a results in a const char (and thus *a, a[5], *(a + 3) etc cannot be modified).

char const* a;

"a is a pointer to a const char."

Exact same thing as the first example.

char * const a;

"a is a constant pointer to a char."

This means that a cannot be changed (a = b; and a = &c; are invalid), but the contents of a can be (a[3] = 'b'; is valid).

const char* const a;

"a is a constant pointer to a constant char."

Neither a nor what it points to can be altered.

Answer 3

1. What's not magic in C is that pointers can be null, and assigning to NULL will cause segfault. The same applies to [] access on char* that are NULL.

2. You've created a function with a return type, but you never return anything. It doesn't matter in your test case, but anyone else calling this could expect a value there. Most compilers will complain.

3. If you are trying to replicate strcat() just to learn, understand its assumptions. It is assuming that the arg to which something is having strcat() performed on it will be big enough not to be overrun, and it's up to the caller to prepare against this. If you are trying to write something safer, then you need to figure out how to handle the error conditions (e.g. since you're using a third arg here, do you want to return some kind of BOOL indicating a good strcat() took place (i.e. that all the args were good and non-NULL?)).