C returning const char pointer or char pointer from a function

Question

I wanted to understand this a bit better so I am asking here. I write a function that reads a file and returns the contents as a string. It is currently implemented as returning a char* because it seems easier but I am wondering if this is the correct approach as a lot of C function prototypes that consumes char arrays consume them as const char. The reason that I say it's easier is because once you've read all the data if you want to return a const char, I have to create a new buffer that's the exact size and copy the data over there, instead of just reallocating the buffer down to the correct size and returning the pointer that was allocated on the heap.

My question, should the return value be a const char* or char*?

Here's a little code:

char *get_resource(char **res, const char *filename) {
    size_t count = ((strlen(resource_dir) + strlen(filename) + 1));
    *res = calloc(count, sizeof(char));
    strncpy(*res, resource_dir, resource_dir_len);
    strncat(*res, filename, strlen(filename));
    return *res;
}

or this one:

char *read_file(char **data, const char *file_path) {
    FILE *fp;
    size_t buffer = 4096;
    size_t index = 0;
    int ch;

    fp = fopen(file_path, "r");
    if (fp == NULL) {
        printf("failed to open file: %s\n", file_path);
        return "-1\0";
    }

    (*data) = calloc(buffer, sizeof(char));
    while (EOF != (ch = fgetc(fp))) {
        (*data)[index] = (char)ch;
        ++index;
        if (index == buffer - 1) {
            buffer = buffer * 2;
            data = realloc(data, buffer);
            if (data != NULL) {
                printf("buffer not large enough, reallocating %zu bytes to "
                       "load %s\n",
                       buffer, file_path);
            } else {
                printf("failed to realloc %zu bytes to load %s\n", buffer,
                       file_path);
            }
        }
    }
    (*data) = realloc((*data), (sizeof(char) * (index + 1)));
    (*data)[index] = '\0';

    fclose(fp);
    return *data;
}

The return value of second code should be `const char*` because a pointer converted from string literal may be returned from the function. — MikeCAT, Feb 27 '16 at 15:18
The first code snippet uses `strncpy` / `strncat` incorrectly. The `strncat` function only works if the first argument points to a null-terminated string, but `strncpy` does not null terminate the way you have used it (assuming `resource_dir_len == strlen(resource_dir)`). Suggest never using `strncpy`, instead you can use `strcpy` or `snprintf`. — M.M, Feb 27 '16 at 15:20
Why are you returning the pointer to the result buffer twice: via the first argument and via the return value? You can pass a `char*` to a function requesting `const char*`, but not vice versa. (Maybe, I misunderstood your question.) — Martin Zabel, Feb 27 '16 at 16:33
I see posts saying to use strncpy because it's safer then I see not to use strncpy because it's not. These things get a bit hard to keep up with sometimes, from the bolded statement above I think that I shouldn't be using it so I'll change that. @MartinZabel I do it that way so that I can do multiple function calls in line. The reason I pass in the variable that gets returned is so that I don't get memory leaks while doing in line functions. — user1610950, Feb 27 '16 at 18:02
I think the other guys have overlooked that the memory was allocated with `calloc`, and me too. Thus the buffer already contained the terminating NUL character, required by `strncat` and later on. — Martin Zabel, Feb 27 '16 at 19:10
@user1610950: for detailed explanations regarding `strncpy`, see this post: http://stackoverflow.com/questions/869883/why-is-strncpy-insecure and this answer: http://stackoverflow.com/a/1258577/4593267 . This function is very poorly understood and not very useful. It is very error prone as most programmers misuse it. In your case, it is very likely misused and the `strncat` below it is exactly equivalent to `strcat(*res, filename);`. — chqrlie, Feb 28 '16 at 12:09
@user1610950: also read this very good article by Keith Thompson: http://the-flat-trantor-society.blogspot.fr/2012/03/no-strncpy-is-not-safer-strcpy.html — chqrlie, Feb 28 '16 at 12:14
thank you for all the info on strncpy/ strcpy. I'll be sure to implement it in my projects going forward. — user1610950, Mar 05 '16 at 05:59

Frankie_C · Accepted Answer · 2016-02-27T18:34:02.203

There are some errors already explained in comments that I'll not repeat, but that I suggest you to fix asap.
Now you have to consider when typing a definition what's the expected or required behavior toward data from inside and from outside the function.
In your case you need a changeable data buffer inside the function, but an unchangeable data buffer outside. Clarified this point you can type the function consequently and then use casting:

const char *get_resource(const char** InRes, const char* filename)
{
    char *res;
    size_t count = ((strlen(resource_dir) + strlen(filename) + 1));
    if (*InRes)
    {
        res = realloc((void *)*InRes, count * sizeof(char));
        if (!res)
            return NULL;
    }
    else
    {
        res = calloc(count, sizeof(char));
        if (!res)
            return NULL;
    }
    *InRes = res;
    strcpy(res, resource_dir);
    strcat(res, filename);
    return res;
}

Note on the return where the cast is not required because making const the data is not a violation of the data nature as could be the reverse (e.g. make changeable constant data).

Why the initialization of `res` at its declaration? `res` is updated in the `calloc` line anyway. And the incoming `const char *` is not updated via `*InRes` anymore. — Martin Zabel, Feb 27 '16 at 18:09
@MartinZabel Thanks Martin. I missed it. I have update the sample with code that updates the source buffer. If `InRes` exists before it is reallocated. — Frankie_C, Feb 27 '16 at 18:35

C returning const char pointer or char pointer from a function

1 Answers1