CODEWITHSUNDEEP
cparsinggetopt
Telefang
Telefang

Reputation: 133

Parsing arguments in C using getopt

I'm trying to parse the arguments passed to my program to use them in a function. The format of the arguments is:

emp ­­-p one.txt two.txt three.bin ­-­o file.emp

I want to save the the arguments after -p in char **arg_pack and the arguments after -o in char **arg_output.

Here is part of my code:

#define OPTION_STRING "p:u:r:l:d:o:i:ah"
#define OP_ARG_PACK        'p'

extern char *optarg;
extern int optind, opterr, optopt;

char **arg_pack;
char **arg_output;

int arg_pack_count = 0;
int arg_output_count = 0;

while ((res = getopt(argc, argv, OPTION_STRING)) != -1 )
{
    if (res == (int) OP_ARG_PACK)
    {   
        int index = optind - 1;
        arg_pack_count++;
        while(index < argc)
        {   
            *arg_pack = malloc(strlen(argv[index])+1 );
            strcpy(arg_pack[index], argv[index]);
            index++;

            if ( (index<argc) && ((argv[index])[0] == '-') )
            {         
                optind = index - 1;
                break;
            }
        }
        emp_pack(arg_pack,arg_output);
    }
}

It's giving me a segmentation fault. Why is that?

Upvotes: 1

Views: 676

Answers (3)

evaitl
evaitl

Reputation: 1395

char **arg_pack;

arg_pack is pointing at NULL. Right here:

*arg_pack = malloc(strlen(argv[index])+1 );

I think this dereferences a null pointer and tries to assign to it.

Try something like 

int arg_pack_max=10;
arg_pack=malloc(arg_pac_max*sizeof(char *))

somewhere and if arg_pack_count gets to arg_pack_max, realloc() and get more space for arg_pack

Upvotes: 1

user3629249
user3629249

Reputation: 16540

regarding this code:

extern char *optarg;
extern int optind, opterr, optopt;

char **arg_pack;
char **arg_output;

int arg_pack_count = 0;
int arg_output_count = 0;

while ((res = getopt(argc, argv, OPTION_STRING)) != -1 )
{
    if (res == (int) OP_ARG_PACK)
    {   
        int index = optind - 1;
        arg_pack_count++;
        while(index < argc)
        {   
            *arg_pack = malloc(strlen(argv[index])+1 );
            strcpy(arg_pack[index], argv[index]);
            index++;

            if ( (index<argc) && ((argv[index])[0] == '-') )
            {         
                optind = index - 1;
                break;
            }
        }
            emp_pack(arg_pack,arg_output);
}

The reason for the seg fault is using pointers (with offset indexs) that do not point to any allocated memory.

the call to malloc() keeps overlaying the pointer (if any) in the variable: char **arg_pack;

the getopt() will only get access to the next parameter after the 'found' option and will not handle invalid options nor the (expected) multiple values after an option.

A much easier (although slightly wasteful) method, to allocate the needed memory is:

char * argpack = NULL;
if (NULL == (argpack = malloc( argc * sizeof(char *) ) ) )
{ // then malloc failed
    perror( "malloc for -p array of pointers failed" );
    exit( EXIT_FAILURE );
}

// implied else, malloc successful

if( NULL == (argoutput = malloc( argc * sizeof( char *) ) ) )
{ // then malloc failed
    perror( "malloc for -o array of pointers failed" );
    free( argpack );
    exit( EXIT_FAILURE );
}

// implied else, malloc successful

while( .... )
{
    .... // use `strdup()` to set the pointers
}

however, that is not all that is wrong with the logic of the posted code.

Suggest: 

size_t argpackIndex = 0;
size_t argoutputIndex = 0;

char lastSeenOption = '\0';

for( int x=1; x<argc; x++ ) // 1 to skip over executable file name
{
    if( '-' == argv[x][0] )
    {
        lastSeenOption = tolower(argv[x][1]);
    }

    else
    {
         switch( lastSeenOption )
         {
             case 'p':
                argpack[ argpackIndex ] = strdup( argv[x] );
                argpackIndex++;
                break;

             case 'o':
                argoutput[argoutputIndex ] = strdup( argv[x] );
                argoutputIndex++;
                break;

             default:
                fprinf( stderr, "ignoring invalid option: <%c> for parameter value: <%s>\n", 
                        lastSeenOption,
                        argv[x] );
                break;
        } // end switch
    } // end if
} // end for

Upvotes: 1

T Johnson
T Johnson

Reputation: 824

According to the Linux man page

By default, getopt() permutes the contents of argv as it scans, so that eventually all the nonoptions are at the end.

so when you try to index into argv inside the while loop you are indexing into an argv that may not be like the argv you started with.

Upvotes: 1

Related Questions