Thread: Need help understanding a piece of code

Hi,

I came across an example program for command line arguments in C and it contains a piece of code I can't get my head around. The following is the program:

Code:

#include <stdio.h>
   #include <stdlib.h>
   #include <string.h>
   
   main( int argc, char *argv[] )
   {
     int m, n,                              /* Loop counters. */
         l,                                 /* String length. */
         x,                                 /* Exit code. */
         ch;                                /* Character buffer. */
     char s[256];                           /* String buffer. */
   
     for( n = 1; n < argc; n++ )            /* Scan through args. */
     {
       switch( (int)argv[n][0] )            /* Check for option character. */
       {
       case '-':
       case '/': x = 0;                   /* Bail out if 1. */
                 l = strlen( argv[n] );
                 for( m = 1; m < l; ++m ) /* Scan through options. */
                 {
                   ch = (int)argv[n][m];
                   switch( ch )
                   {
                   case 'a':              /* Legal options. */
                   case 'A':
                   case 'b':
                   case 'B':
                   case 'C':
                   case 'd':
                   case 'D': printf( "Option code = %c\n", ch );
                             break;
                   case 's':              /* String parameter. */
                   case 'S': if( m + 1 >= l )
                             {
                               puts( "Illegal syntax -- no string!" );
                               exit( 1 );
                             }
                             else
                             {
                               strcpy( s, &argv[n][m+1] );
                               printf( "String = %s\n", s );
                             }
                             x = 1;
                             break;
                   default:  printf( "Illegal option code = %c\n", ch );
                             x = 1;      /* Not legal option. */
                             exit( 1 );
                             break;
                   }
                   if( x == 1 )
                   {
                     break;
                   }
                 }
                 break;
       default:  printf( "Text = %s\n", argv[n] ); /* Not option -- text. */
                 break;
       }
     }
     puts( "DONE!" );
   }

What I don't understand is the code in the second switch statement marked with string parameter. What do I have to input to get to the printf statement: string = %s ? what does the -s option do?

All in all I am confused and I would really appreciate any help that can help me crawl out of the dark state of noobness I'm in now. Thanks.

It is a good example of how buffer overruns can occur. At minimum, it should have used

Code:

strncpy(s, &argv[n][m+1], sizeof s);
s[(sizeof s) - 1] = '\0';

instead, to avoid overrunning the s array (and making sure it will be properly terminated in all cases).

Above n is the index to the current command-line argument, and m is the index of the current option character within that string. So, it will copy whatever follows the option character in the same option.

Try -sfoo or /sfoo command line options to see for yourself.

outer switch looks for a '-' or a '/', indicating an argument. if it sees one, it enters the case
the length of the current argument is taken and assigned to 'l'
the loop iterates over all the characters in the current argument except the initial '-' or '/'
if the first character seen is an 's' or 'S', it enters that case.
if the current index 'm' + 1 is greater than the length of the argument string, then there is no string and it enters the 'illegal' print. in other words, the code looks to see if there are characters immediately following the '-s'. if there are not, it is illegal.
if there are characters following the '-s', then it enters the else clause and copies those characters to the char array variable 's' (with a possible overrun as the length is not checked that it will fit in 's').

so to get it to print "String =...", you would enter ><program name> -sthisisthestring
and it should print out String=thisisthestring.

Thanks a lot guys! I am used to it being a space between the argument and the string, but now I see. Out of curiosity, how would one add a space between '-s' and the string without getting an error? Again thanks, it's nice to see that some forums still are helpful

Comment on the code you found: using lower-case L as a variable name is generally a bad idea, as it is too easily mistaken for the numeral one. In fact, outside of loop constructs, single-letter variable names in general are frowned upon.

Originally Posted by youjustreadthis

I am used to it being a space between the argument and the string

It is more common, yes. Technically, the argument is in the next parameter.

Originally Posted by youjustreadthis

Out of curiosity, how would one add a space between '-s' and the string without getting an error?

If you mean without modifying the program: By supplying the parameter quoted so that the shell will not split them into separate arguments. For example, "-s thishasaspaceinfront" (including the quotes, verbatim).

If you meant to ask how would one need to modify the program so it would accept the parameter from the next command-line argument (as is common), then you could replace the relevant if clauses with

Code:

if( m + 1 >= l )
{
    if( n + 1 >= argc )
    {
        puts("-s: No parameter specified");
        exit(1);
    }

    /* It is considered bad form to modify the for loop variable like this, but we have to. */
    n++;

    strncpy( s, argv[n], sizeof s );
    s[ (sizeof s) - 1 ] = '\0';
}
else
{
    strncpy( s, argv[n], sizeof s );
    s[ (sizeof s) - 1 ] = '\0';
}

printf( "String = %s\n", s );

That worked nicely, but why exactly is it considered bad form if it works? I can see why single letter variable names like l are frowned upon as rags_to_riches pointed out, but not sure if I got what was bad form. Thanks again, you are all being really helpful, despite my thickness haha

Let me reiterate: unless you know differently, it is best to use getopt().

I personally do not, but that might be just because I work with weird people in weird environments. And I am pretty wonky myself.

I just thought my weird approach might be relevant, because the original program at issue here was parsing options. In particular, I intended to emphasize how a while loop yields more predictable/easy-to-read code, than a for loop where the loop variable is occasionally modified also in the loop body.

Incrementing the loop variable in the loop body is necessary when an option has an argument as a separate parameter, as opposed to immediately following the option character (as the original code in this thread expects).

Originally Posted by christop

I'm pretty sure the standard getopt() interface stops processing options on "--".

Now that you mention it, it does and should. It is even explicitly mentioned in the tenth guideline in IEEE Std 1003.1-2001 Utility Argument Syntax chapter.

Originally Posted by christop

This is quite different behavior from every tool I've ever used, which almost all allow an argument immediately after an option letter.

I know. In computing cluster environments, where things are started via noninteractive scripts, having the argument immediately follow the option is problematic, as in many cases previous errors cause the argument to be empty. Not allowing it (and telling the users to avoid that usage) reduces problems.

The rules I described are just a consequence of disallowing immediate arguments to single-character options.

Originally Posted by christop

it's easy enough to unambiguously pass an empty argument to an option using the standard getopt rules.

Yes, but allowing the argument to immediately follow the option causes unexpected behaviour if the argument might be empty. Consider a script, for example, with the argument taken from a shell variable.

I've found warning against and disallowing immediate arguments to single-character options avoids such problems. Especially since users tend to start fixing the first symptom they see, instead of finding out the root cause first.

Originally Posted by christop

Also, there is one partial exception to the rule: tar.

Yes, which also happens to be one of the more common command-line tools in computing cluster environments; i.e. one of the tools my users are most familiar with. Funnily enough, I think I've seen cases similar to tar -cfz foo.tar.gz (followed by "This system must be broken! Where did my data go?") more than once in real life..

Originally Posted by Nominal Animal

I know. In computing cluster environments, where things are started via noninteractive scripts, having the argument immediately follow the option is problematic, as in many cases previous errors cause the argument to be empty. Not allowing it (and telling the users to avoid that usage) reduces problems.

The rules I described are just a consequence of disallowing immediate arguments to single-character options.

I see no problem with disallowing immediate option arguments. It shouldn't cause any problems with new software (only older programs should still accept immediate options for the sake of backwards compatibility). If a user tries to call the program with an immediate argument (eg, "-n2"), the program should complain about an invalid option, preventing their scripts from breaking in the future. I only had a little issue with accepting option arguments that don't follow the option that it corresponds to, eg "-nd 2 /". That's an additional feature that just seems strange and error-prone to me.

Yes, which also happens to be one of the more common command-line tools in computing cluster environments; i.e. one of the tools my users are most familiar with. Funnily enough, I think I've seen cases similar to tar -cfz foo.tar.gz (followed by "This system must be broken! Where did my data go?") more than once in real life..

I was thinking of something an "experienced" sysadmin did at a previous job I had when I wrote about tar. He ended up with a large tar file named "z".

Thanks to both of you for clearing up so many things! did not know a lot of this. I will do as you recomended and use getopt() for my little project as it seems to provide me with exactly what I was looking for. Really appreciate it

Originally Posted by christop

I only had a little issue with accepting option arguments that don't follow the option that it corresponds to, eg "-nd 2 /". That's an additional feature that just seems strange and error-prone to me.

Well, it turns out that when you implement the short option handling as a loop over each character in the short option string, that is just the easiest and simplest way to handle the arguments.

To be honest, I think this might be the first time I've mentioned at all that my option parsing allows -nd 2 / as a shorthand for -n 2 -d / . I don't even know why I blurted it out.. I only show the expanded form in usage and documentation.

I've posted a Bash implementation of the option parsing on another site here.

Originally Posted by christop

I was thinking of something an "experienced" sysadmin did at a previous job I had when I wrote about tar. He ended up with a large tar file named "z".

In some cases it is useful to transfer files directly using an ssh connection and tar at both ends (as opposed to using scp). One out of every ten or so times I still end up with a file named "-" at one end.

Just one more thing regarding optget(), more specifically optget_long(). Why are the flags necessary? as far as I can see they don't do much.

Code:

static struct option long_options[] =
             {
               /* These options set a flag. */
               {"verbose", no_argument,       &verbose_flag, 1},
               {"brief",   no_argument,       &verbose_flag, 0},
               /* These options don't set a flag.
                  We distinguish them by their indices. */
               {"add",     no_argument,       0, 'a'},
               {"append",  no_argument,       0, 'b'},
               {"delete",  required_argument, 0, 'd'},
               {"create",  required_argument, 0, 'c'},
               {"file",    required_argument, 0, 'f'},
               {0, 0, 0, 0}

Code:

/* Instead of reporting ‘--verbose’
          and ‘--brief’ as they are encountered,
          we report the final status resulting from them. */
       if (verbose_flag)
         puts ("verbose flag is set");

The man 3 getopt_long man page is your friend. In particular:

flag

specifies how results are returned for a long option. If flag is NULL, then getopt_long() returns val. (For example, the calling program may set val to the equivalent short option character.) Otherwise, getopt_long() returns 0, and flag points to a variable which is set to val if the option is found, but left unchanged if the option is not found.

If you really need to have a value of -1, ':', or '?', you need to use the flag pointer instead of having getopt_long() return that value. getopt_long() will return -1 when it is done, and ':' or '?' if it encounters an unknown or ambiguous option.

I think the true reason for the flag pointer in the structure is to allow different approches, however. This way developers can freely pick whether they want to handle each value in a separate location, or as a return value from the getopt_long() function. It has very little cost -- it's not like you're going to have thousands and thousands of options, so an extra pointer to an int per option is a small cost --, but it potentially allows for much simpler code in some cases.

As a developer, I like being able to choose the best approach, and not just forced into a random one by a library function.