0

I am trying to do an RPN where my not constant argv vector stores and retrieves results of calculations by replacing an operator with the result of the operation: argv[0]="2"; argv[1]="3"; argv[2]="+" ==> argv[0]="2";argv[1]="3";argv[2]=5 My code works for simple RPN input -- input with a maximum of one operation; it fails at nested operations because when it reads argv[n] it finds nothing in it, so it sees it as zero. Can you guys help? Here's the code:

    /*
    * Ahmed AlJehairan
    * Github: aj326
    * Description: Reverse Polish Calc. Usage ./expr n1 n2 op
    */
    #include <stdio.h>
    #include <stdlib.h>
    #define DEBUG 1
    //Will implement error checking later:
    /*
    Errors to check for:
       arg starts with letter
       first arg not a number
       check at least two arg for binary ops
    */

    int main(int argc, char *argv[])
    {
        char **s = argv;
        int len = argc;
        if (argc < 3)
        {
            printf("At least 2 arguments\n");
            return 1;
        }
    if (--argc)   argv++;

    int val;
    while (argc--)
    {
        switch (**argv)
        {

        case '+': ; val = atoi(*(argv - 2)) + atoi(*(argv - 1)); snprintf (*argv, sizeof(int), "%d", val); if (DEBUG) printf("after arith *argv %s \n", *argv); break;
        case '=': ; val = atoi(*(argv - 2)) == atoi(*(argv - 1)); snprintf (*argv, sizeof(int), "%d", val); if (DEBUG) printf("after arith *argv %s \n", *argv); break;
        case '-': ; val = atoi(*(argv - 2)) - atoi(*(argv - 1)); snprintf (*argv, sizeof(int), "%d", val); if (DEBUG) printf("after arith *argv %s \n", *argv); break;
        case '*': ; val = atoi(*(argv - 2)) * atoi(*(argv - 1)); snprintf (*argv, sizeof(int), "%d", val); if (DEBUG) printf("after arith *argv %s \n", *argv); break;
        case '/': ; val = atoi(*(argv - 2)) / atoi(*(argv - 1)); snprintf (*argv, sizeof(int), "%d", val); if (DEBUG) printf("after arith *argv %s \n", *argv); break;
        case '%': ; val = atoi(*(argv - 2)) % atoi(*(argv - 1)); snprintf (*argv, sizeof(int), "%d", val); if (DEBUG) printf("after arith *argv %s \n", *argv); break;
        }
        if (DEBUG)
        {
            int i;
            for (i = 0; i < len; ++i)
            {
                printf("%s\t", s[i]);
            }
            printf("\n");
        }
        argv++;
    }
    printf("%s\n", *--argv);
    return 0;
}
  • To be prepared an array of another conversion for the result. – BLUEPIXY Sep 27 '14 at 01:00
  • 1
    I'll simply note the abject horridness of writing a program that invokes undefined behavior *by design* given non-valid input. I.e. `+ 0 1`. Worse, assuming input is "valid", though the standard allows you to store *alternate* addresses at `argv[n]` of any `n` in 0..argc. It makes no such claim that memory at the *exiting* addresses is writable. In short, this is a terrible idea. Use a properly sized local `int` stack. – WhozCraig Sep 27 '14 at 01:37
  • @WhozCraig Concerning "standard allows you to store alternate addresses at argv[n]" see http://stackoverflow.com/questions/25737434/is-argvn-writable/25747537 Also the spec explicitly says argv[][] is writable though it is quiet on the range. – chux - Reinstate Monica Sep 27 '14 at 03:00
  • 1
    @chux new to me. As I recall (admittedly potentially incorrectly), the strings pointed to by argv were not always modifiable, though the addresses *in* `argv` were replacable. However, even in the OP's case, the size of a given string single-char param `'1'` is not `sizeof(int)` (32bit = 4 octets). At most it would be `1` writable char + the terminating NULL (which the second param to `snprintf` should account for; i.e. 2 means 1+nullchar, 4 means 3+nullchar, which clearly won't fit in a string previously 1+nullchar). – WhozCraig Sep 27 '14 at 03:04

1 Answers1

1

snprintf (*argv, sizeof(int),... is not necessarily valid.

The size available it certainly not defined by sizeof(int) and should not assumed to be any more than the original strlen(argv[])+1.

The size of char needed to print an arbitrary int varies. Suggest

#define MAXPRINTSIZE (CHAR_BIT*sizeof(int)*10/33 + 3)
char bufer[MAXPRINTSIZE];
snprintf (buffer, sizeof buffer, "%d", val); 

You will have to re-think your buffer management.

Community
  • 1
  • 1
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256