HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

C-string manipulation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
manipulationstringstackoverflow

Problem

I'd just like to get some pointers on my newbie-ish C here. The intention is to have an environment variable provide the "prefix" for any paths this app needs.

For example, APP_PREFIX could be /opt/app, a path the app wanted to get could be /etc/app.conf, and so app_path("/etc/app.conf") should return /opt/app/etc/app.conf.

I realise this is currently sensitive to leading/trailing slashes; I just wanted to make sure I'm on the right track with this basic stuff. Be as pedantic as you like – from outright bugs down to style and convention.

It compiles with no warnings on Mac OS X with gcc -Wall -pedantic ..., and does what I expect it to. On Linux, I had to compile with libbsd, and #include for strlcpy() and strlcat().

#include 
#include 
#include 

#define APP_PREFIX_ENV_VAR "APP_PREFIX"

const char * app_path_prefix() {
    char * env_prefix = getenv(APP_PREFIX_ENV_VAR);
    if (env_prefix == NULL) {
        return "";
    } else {
        return env_prefix;
    }
}

char * app_path(char * in_path) {
    const char * prefix = app_path_prefix();
    size_t prefix_len = strlen(prefix);
    size_t out_size = prefix_len + strlen(in_path) + 1;
    char * out = malloc(out_size);

    strlcpy(out, prefix, prefix_len + 1);
    strlcat(out, in_path, out_size);
    return out;
}

int main(int argc, char ** argv) {
    char * path = app_path(argv[1]);
    printf("result: '%s'\n", path);
    free(path);
    return EXIT_SUCCESS;
}

Solution

All seems good.

As you mentioned it is sensitive to trailing slash being missing. I would just force that issue and always append one between the prefix and the path. An extra slash will not hurt but a missing one will.

Apart from not using C in the first place :-) all my other issues are simple stylistic ones that only 30% of people would agree (I always divide stylistic camps as 30% Side A, 30% side ~A, 40% don't care).

A tiny bit more white space to make it easier to read. But let me just say your code is not bad in that regards (just slightly less than I would use).

I don't want to recommend any particular style. But I can show you how I would have written it:

// Keep you includes sorted in alphabetical order
// Here it is not such a big deal but with big projects the list can get big
// and alphabetical order helps you find things in large lists.
#include 
#include 
#include 

#define APP_PREFIX_ENV_VAR "APP_PREFIX"

const char * app_path_prefix()
{
    char * env_prefix = getenv(APP_PREFIX_ENV_VAR);
    return (env_prefix == NULL)
               ? ""
               : env_prefix;
}

char * app_path(char * in_path)
{
    /*
     * Some people really hate this.
     *
     * Others like myself really like lining up the '=' sign
     * Just do it locally with a set of calculations. I find it easier to see
     * both the lhs and the rhs of expression quickly.
     */
    char const* prefix     = app_path_prefix();
    size_t      prefix_len = strlen(prefix);
    size_t      out_size   = prefix_len + strlen(in_path) + 1;

    /* 
     * Dynamically allocated. Released to outside world it is the
     * responsibility of the caller to free this memory
     */
    char *      out        = malloc(out_size);

    strlcpy(out, prefix, prefix_len + 1);
    strlcat(out, in_path, out_size);
    return out;
}

int main(int argc, char ** argv)
{
    /* test */
    char * path = app_path(argv[1]);
    printf("result: '%s'\n", path);
    free(path);

    return EXIT_SUCCESS;
}

Code Snippets

// Keep you includes sorted in alphabetical order
// Here it is not such a big deal but with big projects the list can get big
// and alphabetical order helps you find things in large lists.
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define APP_PREFIX_ENV_VAR "APP_PREFIX"

const char * app_path_prefix()
{
    char * env_prefix = getenv(APP_PREFIX_ENV_VAR);
    return (env_prefix == NULL)
               ? ""
               : env_prefix;
}

char * app_path(char * in_path)
{
    /*
     * Some people really hate this.
     *
     * Others like myself really like lining up the '=' sign
     * Just do it locally with a set of calculations. I find it easier to see
     * both the lhs and the rhs of expression quickly.
     */
    char const* prefix     = app_path_prefix();
    size_t      prefix_len = strlen(prefix);
    size_t      out_size   = prefix_len + strlen(in_path) + 1;

    /* 
     * Dynamically allocated. Released to outside world it is the
     * responsibility of the caller to free this memory
     */
    char *      out        = malloc(out_size);

    strlcpy(out, prefix, prefix_len + 1);
    strlcat(out, in_path, out_size);
    return out;
}

int main(int argc, char ** argv)
{
    /* test */
    char * path = app_path(argv[1]);
    printf("result: '%s'\n", path);
    free(path);

    return EXIT_SUCCESS;
}

Context

StackExchange Code Review Q#9037, answer score: 3

Revisions (0)

No revisions yet.