2
\$\begingroup\$

I have written a program in C, which does the following operations:

  • creates or splits Camel Case variable, method, class names.

Input format

  • Each line of the input file will begin with an operation (S or C) followed by a semi-colon followed by M, C, or V followed by a semi-colon followed by the words you'll need to operate on.

  • The operation will either be S (split) or C (combine)

  • M indicates method, C indicates class, and V indicates variable

  • In the case of a split operation, the words will be a camel case method, class or variable name that you need to split into a space-delimited list of words starting with a lowercase letter.

  • In the case of a combine operation, the words will be a space-delimited list of words starting with lowercase letters that you need to combine into the appropriate camel case String. Methods should end with an empty set of parentheses to differentiate them from variable names.

Output Format:

  • For each input line, your program should print either the space-delimited list of words (in the case of a split operation) or the appropriate camel case string (in the case of a combine operation).

Sample Input

S;M;plasticCup()
C;V;mobile phone
C;C;coffee machine
S;C;LargeSoftwareBook
C;M;white sheet of paper
S;V;pictureFrame

Sample Output

plastic cup
mobilePhone
CoffeeMachine
large software book
whiteSheetOfPaper()
picture frame

here is what I've written so far:

#include <asm-generic/errno-base.h>
#include <asm-generic/errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
#include <stddef.h>
#include <errno.h>
#include <err.h>
#include <sys/types.h>

#define SPACE       ' '
#define SPLIT       'S'
#define COMBINE     'C'
#define METHOD      'M'
#define CLASS       COMBINE
#define VARIABLE    'V'

static char *caseProblem(char *restrict);
static void *substr(const void *restrict, size_t);
static char *split(char *restrict, int);
static char *combine(char *restrict, int);
static void *__memmove(void *restrict, int, size_t);

int
main(void)
{
    char *p = NULL;
    ssize_t len;
    size_t n = 0;

    while ((len = getline(&p, &n, stdin)) != -1)
        (void)fprintf(stdout, "%s", caseProblem(p));

    free(p);
    exit(EXIT_SUCCESS);
}


static void *
__memmove(void *restrict src, int delim, size_t n)
{
    char *dest = NULL;

    if ((dest = (char *)malloc(65535 * sizeof(char))) == NULL)
        errx(EXIT_FAILURE, "%s", strerror(ENOMEM));

    memmove(dest, src, strlen(src) - n);
    dest[strlen(dest)] = delim;
    return (dest);
}


static char *
caseProblem(char *restrict p)
{
    char opt, type;
    unsigned int i;
    size_t size = strlen(p);

    /* extract option (S or C) */
    opt  = p[0];
   /* extract type (M, V or C) */
    type = p[2];

    p = substr(p, 4);
    if (opt == SPLIT && type == METHOD)
        p = __memmove(p, '\n', 3);
    
    if (opt == COMBINE && type == METHOD) {
        p = __memmove(p, 0, 1); // reject: '\n'?
        strcat(p, "()\n");
    }

    i = 0;;
    while (i < size) {
        
        switch (opt) {

        case SPLIT:
            p = split(p, type);
            break;

        case COMBINE:
            p = combine(p, type);
            break;

        default:
            errx(EXIT_FAILURE, "Invalid Option...!\n");
        }   
        ++i;
    }
    p[i] = '\0';    
    return (char *)(p);
}


static void *
substr(const void *restrict src, size_t n)
{
    char *dest = NULL;
    
    if ((dest = (char *)malloc(65535 * sizeof(char))) == NULL)
        errx(EXIT_FAILURE, "%s", strerror(ENOMEM));

    return (memcpy(dest, (src + n), strlen(src)));
}


static char *
split(char *restrict in, int type)
{
    char *out = NULL;
    unsigned int i, o;
    size_t size = strlen(in);

    if ((out = (char *)malloc(65535 * sizeof(char))) == NULL)
        errx(EXIT_FAILURE, "%s", strerror(ENOMEM));

    i = o = 0;
    while (i < size) {
        if (type == METHOD || type == VARIABLE || type == CLASS) {
            if (isupper(in[i])) {
                in[i] = tolower(in[i]); 

                if (i != 0) {
                    out[o++] = SPACE;
                    out[o] = in[i];
                }
            }
        } else {
            errx(EXIT_FAILURE, "Invalid Type...!\n");
        }
        out[o++] = in[i];
        ++i;
    }
    out[o] = '\0';
    return (char *)(out);
}


static char *
combine(char *restrict p, int type)
{
    unsigned int i, j, space;
    size_t size = strlen(p);

    i = j = 0;
    space = 0;
    while (i < size) {
    
        switch (type) {

        case VARIABLE:
        case METHOD:    
            if (p[i] == SPACE) {
                ++space;

                if (islower(p[space]))
                    p[i + 1] = toupper(p[i + 1]);
            }
            break;
        
        case CLASS:
            if (i == 0)
                p[i] = toupper(p[i]);
            
            if (p[i] == SPACE) {
                ++space;
            
                if (islower(p[space]))
                    p[i + 1] = toupper(p[i + 1]);
            }
            break;

        default:
            errx(EXIT_FAILURE, "Invalid Type...!\n");
            break;

        }

        if (p[i] != SPACE)
            p[j++] = p[i];
        ++i;
    }
    p[j] = '\0';

    return (p);
}

I'd like to know if there is any way to improve it.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Memory leaks and unclear ownership

caseProblem's use of __memmove will lead to memory leaks, as it always overwrites the value of p.

Use of getline's internal allocation I feel is unclear and leads to the other problems, because its not clear who 'owns' and is responsible for the initial allocation.

Whilst the C Library is literred with examples of internal allocation, its generally not good practice. It's good practice for the caller to be responsible for allocation, which could be either heap or stack depending on the size of memory required (and in some contexts dynamic allocation can ultimately lead to failure due to fragmentation).

The C library has got better over time (whilst carrying the technical debt of earlier bad practices) and it usual to require a method to take both the source and destination address AND the size allocated (or used) by each, so that the method can enforce rules regarding overrun and overlap if required.

Fix memory allocations could lead to overflow or excess memory usage

Both substr and __memmove are allocating 65k of memory regardless of the input length. It should be clear when functions are allocating memory, usually by providing a specialise xxx_free function. Alternatively, avoid doing memory allocation within subroutines, allowing the caller to provide the destination address, which can then be memory allocated from stack or heap according to its needs.

Function naming

__memmove is poorly named. To avoid a conflict with the standard library memmove you've added double underscore, which is a convention to indicate a private and internal API. You've also changed the semantics in the process, especially regards the internal allocation, which guarantees that memcpy could have been used (as destinstation will never overlap source).

Personally I'd find a completely different name that more accurately describes what its doing (e.g. strncpyWithDelimiter).

It's not a great name for the standard library as it happens, it differs from memcpy only in that it allows for the source and destination addresses to overlap. But then the standard C is litered with poorly named functions in part due to its age.

substr seems to be doing almost exactly the same but without the delimiter.

caseProblem assumptions

This function assumes that the buffer p contains a certain number of characters. It's possible that the allocation is less than the 4 required for the input parameter (for example if the user just presses carriage return). This means that you are dipping into memory over which you have no knowledge of the contents and could lead to indeterminate behaviour. Always check that your buffer is at least the expected length before subscripting (you could use a strlen check at the top of the function).

I've not checked the actual logic used, I think these are the significant problems as it stands.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.