Return string (or char *) from a C function

Question

I need a function to return a string of various sizes. Here is my code:

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

char *get_option_argument(char *arg, char *shortopt, char *longopt) {
    int len = strlen(arg) ;
    size_t len_shortopt = strlen(shortopt) ;
    size_t len_longopt = strlen(longopt) ;

    // we add an extra space to provide the case when shortopt is included in longopt
    char shortopt2[len_shortopt + 1] ;
    char longopt2[len_longopt + 1] ;

    strcpy(shortopt2, shortopt) ;
    shortopt2[len_shortopt] = ' ' ;
    strcpy(longopt2, longopt) ;
    longopt2[len_longopt] = ' ' ;

    // previous version who doesn't work
    /* strcat(shortopt2, shortopt) ;
    strcat(shortopt2, " ") ;
    strcat(longopt2, longopt) ;
    strcat(longopt2, " ") ; */

    // printf("%s, %s\n", shortopt2, longopt2) ;

    int shortopt_boucle = 0 ;
    int longopt_boucle = 0 ;
    int find_shortopt = 0 ;
    int find_longopt = 0 ;
    int j = 0 ;
    int k = 0 ;

    // We browse arg until we reach the end or find an option.
    for (int i=0; i<len && !(find_shortopt || find_longopt); i++) {
        // if we are not already browsing and we have found a potential start for one
        if (!shortopt_boucle && arg[i] == shortopt2[0]) {
            shortopt_boucle = 1 ;
            j = 0 ;
        }
        // same for longopt
        if (!longopt_boucle && arg[i] == longopt2[0]) {
            longopt_boucle = 1 ;
            k = 0 ;
        }
        // if we are procuring a potential option, we check the current character
        if (shortopt_boucle && j <= len_shortopt)
            if (arg[i] == shortopt2[j])
                j++ ;
            else
                j=0 ;
        // idem
        if (longopt_boucle && k <= len_longopt)
            if (arg[i] == longopt2[k])
                k++ ;
            else
                k=0 ;
        // if we have found one, then stop the loop and use i or j variable to mark the start of the option
        if (j == len_shortopt + 1)
            find_shortopt = 1, j = i+1 ;
        if (k == len_longopt + 1)
            find_longopt = 1, k = i+1 ;
    }

    char *result = malloc(sizeof(char)*len) ; // on utilise une allocation dynamique pour pouvoir le retourner

    // get the word just after the option marker
    if (find_shortopt)
        for (int i=0; arg[j+i] != ' ' && arg[j+i] != '\0' && j+i < len ; i++)
            result[i] = arg[j+i] ;
    else if (find_longopt)
        for (int i=0; arg[k+i] != ' ' && arg[k+i] != '\0' && k+i < len ; i++)
            result[i] = arg[k+i] ;

    return result ;
}

void affiche_vache(char *arg) {
    char *eyes = get_option_argument(arg, "-e", "--eyes") ;
    char *hat = get_option_argument(arg, "-h", "--hat") ;
    if (strlen(eyes)!=2)
        eyes = "oo" ;
    if (strlen(hat)!=6)
        hat = " ^__^ " ;
    char *corp1 = "____" ; // this part is for a next implementation
    char *corp2 = "    " ;
    char *corp3 = "----" ;
    printf("         \\ %s\n", hat) ;
    printf("          \\ (%s)\\_%s__\n", eyes, corp1) ;
    printf("            (__)\\ %s  )\\/\\\n", corp2) ;
    printf("                ||%sw |\n", corp3) ;
    printf("                ||     ||\n") ;
    free(eyes) ;
    free(hat) ;
}

int main() {
    affiche_vache("-e 00 -h _|@#|_") ;
    return 0 ;
}

the function get_option_argument must extract an option from a given string. For example, get_option_argument("-e $$", "-e", "--eyes") must return "$$" and does when I call it for the first time, but then it goes wrong.

I think the problem is about memory. I first use this way to declare my string: char result[size] but it cause a segmentation fault and the following warning: function returns address of local variable. That's why I use malloc, but seems like it doesn't really work. Does malloc allocate memory in a different place each time? And the lines with free are commented out because it's provoked segmentation fault.

I apologize for my noisy example, I try to reproduce the same situation with simpler code, but I don't really understand when it's working or not.

Answer 1

You have the following code to allocate space for your options:

char shortopt2[len_shortopt + 1] ;
char longopt2[len_longopt + 1] ;

strcat(shortopt2, shortopt) ;
strcat(shortopt2, " ") ;
strcat(longopt2, longopt) ;
strcat(longopt2, " ") ;

But the scond call to strcat is overwriting the null terminator in the arrays, which will allow further code to walk off into the sunset. You need to replace the above four lines with:

strcpy(shortopt2, shortopt);
strcpy(longopt2, longopt);

I can't figure out exactly what those loops in the following code are doing but I suspect you could again go past the end of either string as you are using the length of the array is the terminal value in the for loops, rather than length - 1. You may need to run this through the debugger to see exactly where it is failing and which variables are the cause.

Also note Yano's comment about the early free of your malloc'ed arrays.

Answer 2

Your code invokes undefined behaviours from the first lines.

shortopt2 and longopt2 are uninitialized and strcat requires a valid C sting as the destination. Also they are too short to accommodate the space " "

    char shortopt2[len_shortopt + 2] ;
    char longopt2[len_longopt + 2] ;

    strcpy(shortopt2, shortopt) ;
    strcat(shortopt2, " ") ;
    strcpy(longopt2, longopt) ;
    strcat(longopt2, " ") ;

Also strlen returns size_t not int.

I didi not analyze the rest of the code

Answer 3

I figure it out! In fact, the bug comes from the strcat, Thanks to @OldBoy I add this: printf ("%s, %s\n", shortopt2, longopt2); in my code just after the concerned line to check, and it had shown this: After calling

char *eyes = get_option_argument(arg, "-e", "--eyes") ;
char *hat = get_option_argument(arg, "-h", "--hat") ;

the code prints successively,

-e , --eyes 
-e -h , --eyes --hat

Meaning that for an obscure reason (there is certainly a good reason for that but for me it's incomprehensible), the variable shortopt2 and longopt2 keeps it content. So that's why the function doesn't find anything during the second call. So I modify

strcat(shortopt2, shortopt) ;
strcat(shortopt2, " ") ;
strcat(longopt2, longopt) ;
strcat(longopt2, " ") ;

to

strcpy(shortopt2, shortopt) ;
shortopt2[len_shortopt] = ' ' ;
strcpy(longopt2, longopt) ;
longopt2[len_longopt] = ' ' ;

And now it's work perfectly !

But if someone has an idea of why these variables keep their content...