Command Line Arguement, can't get it to read in an int

[MaestroRage]

I am trying to get the main to read in a number, and use that number to do a mod calculation. However my best efforts are giving me garbage values. I am sure I am not using the pointer correctly, yet my efforts have gone unrewarded. How can I get this program to take in a number and see if it divisible by 5?

#include <stdio.h>
#define POPPRICE 50 /*what the pop price is in cents*/

char c;
int leftover=POPPRICE; //
int value; // what was entered

int main(int argc, char *argv[])
{
        value=(int)argv[1];
        if ((value / 5) != 10)
        {
                printf("Please enter a number of multiples of 5\n");
                printf("%d\n",value);
        }
        else
        {
        printf("The price of pop is %d, cents", POPPRICE);
        printf("Please insert any combination of nickels [N or n],\n");
        printf("dimes [D or d] or quarters [Q or q].\n");
        printf("You can also return your money to you by pressing R or r\n");
        while ((c=getchar())!='e' && (c=getchar())!='E')
        {
                printf("We have not yet exited\n");
                scanf("%d",&value);
        }
}

}

[Aia]

value=(int)argv[1]; That will not give you what you want. argv[1] is a string in the best case.

Validate that the user entered an argument at the command line. Then use sscanf() to read that string into an int previously declared.

if ( sscanf( argv[1], "%d", &integer ) == 1 )
{
     printf( "%d\n", integer );
}

[Narue]

>Then use sscanf() to read that string into an int previously declared.
If you're already validating the string, sscanf is overkill. If it's a valid integer you can use atoi safely. Or better yet, you can combine the conversion and the validation with strtol:

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

int main ( int argc, char *argv[] )
{
  if ( argc > 1 ) {
    long value;
    char *end;

    errno = 0;
    value = strtol ( argv[1], &end, 0 );

    if ( *end == '\0' && errno == 0 )
      printf ( "Good integer: %ld\n", value );
    else
      fputs ( "Invalid integer format\n", stderr );
  }

  return 0;
}

[Aia]

>If you're already validating the string, sscanf is overkill.
Would you mind to elaborate in what way this would be overkill?

#include <stdio.h>

int main ( int argc, char *argv[] ) {
    int integer =  0;

    if ( argc > 1 ) {
        if ( sscanf( argv[1], "%d", &integer ) == 1 ) {
           <snip>
        }
        else {
           <snip>
        }
    }
    <snip>
    return 0;
}

[jephthah]

here's the simple answer:

int main (int argc, char *argv[])
{
    int value;

    value = atoi(argv[1]);

    ...

}

of course that doesn't even begin to address error checking. But that wasn't your question, so I'm leaving it that out in favor of the simple answer as to why your code didnt work (because you cant merely cast a string as an int)

now, lets the fight resume. i got 3:1 on narue. she wields a wicked chainsaw. i still have scars.

.

[Narue]

>Would you mind to elaborate in what way this would be overkill?
Gladly. I misread your post.

>here's the simple answer:
In this case I'd say that simplicity isn't worth it. You're teaching bad habits again.

>of course that doesn't even begin to address error checking.
Indeed. You make several assumptions about your data:

1) You assume that argv[1] exists.
2) You assume that argv[1] is not a null pointer.
3) You assume that argv[1] represents a valid integer.

Simple or not, those assumptions make your code severely broken because you simply can't control the data sent to your program from an outside source. At the very least you need to correct those three things:

int main ( int argc, char *argv[] )
{
  /* Make sure argv[1] exists and is not null */
  if ( argc > 1 ) {
    /* Make sure argv[1] is a valid integer */
    if ( try_parse_int ( argv[1] ) ) {
      int value = atoi ( argv[1] );

      /* ... */
    }
  }

  return 0;
}

The problem with atoi is that it's literally impossible to error check it. The return value is the only guaranteed mechanism for determining an error, and the value that atoi returns on error could also be returned on success. Not to mention that if an integer can't hold the result, the behavior is undefined. So really the only way to use atoi safely is to write or acquire a validation function for integers:

#include <ctype.h>
#include <limits.h>

int try_parse_int ( const char *s )
{
  /* Base of the conversion */
  const unsigned base = 10;

  /* Largest possible value without the least significant digit */
  const unsigned limit = UINT_MAX / base;

  /* Least significant digit from the largest possible value */
  const unsigned top_digit = UINT_MAX % base;

  unsigned overflow = 0; /* True if integer overflow occurs */
  unsigned sign = 0;     /* Sign of the converted value */

  unsigned temp = 0;     /* The intermediate converted value */
  unsigned n = 0;        /* Count of converted digits */

  /* Save and skip over the sign if present */
  if ( *s == '-' || *s == '+' )
    sign = *s++ == '-';

  /* Build the intermediate value */
  for ( ; isdigit ( *s ); s++, n++ ) {
    unsigned digit = *s - '0';
    
    overflow =  temp > limit
      || ( temp == limit && digit > top_digit );

    if ( overflow )
      break;

    /* Shift-add by the base */
    temp = temp * base + digit;
  }

  if ( n > 0 ) {
    /* A conversion was made; check for overflow */
    if ( overflow
      || ( sign && temp > -INT_MIN )
      || ( !sign && temp > INT_MAX ) )
    {
      /*
        The intermediate actually overflowed,
        or converting it to int would overflow.
        Either way it's an error to the caller
      */
      return 0;
    }
  }

  return n > 0 && *s == '\0';
}

As you can see, there's a hell of a lot more to it than just making sure each character is a digit. You have to consider the sign and the range of the result before the string is guaranteed to be safe for atoi. Though of course, if you're going to all of this work to validate the string, you might as well convert it at the same time. strtol does that for you, which is why I keep saying that atoi is like gets in that it should be completely ignored.

>But that wasn't your question, so I'm leaving it that out in favor of the simple answer
It doesn't matter if it wasn't the question. We're here not only to answer questions, but to help people become better programmers. That means pointing out unrelated problems and correcting bad habits.

[jephthah]

i'm totally down with everything you're saying. and, technically speaking, you're absolutely right.

i also see value in aia's approach.

MY point was that

(1) you can't do this: value=(int)argv[1]; (2) you must do this: value = atoi(argv[1]); (or similar, with strtol)

the poster obviously has a fundamental misunderstanding of how to get an int from command line argument.

now the place you're going with all your bulletproof errorchecking is just going to confuse the guy until he at least understands how to convert a char * to an int.

i dont think it's helpful to hand a beginner a fully fleshed out production-quality routine if they don't understand the basics.

i say let them send invalid args to the commandline and see what happens. otherwise they're just cutting and pasting your Beautiful Code without understanding what it really does.

[Narue]

>i dont think it's helpful to hand a beginner a fully fleshed out production-quality routine
I agree. However, I also don't think it's helpful to dumb down your examples to the point of being blatantly wrong. My post was more for you than for him.

>i say let them send invalid args to the commandline and see what happens.
In which case it might work still "work", if the undefined behavior doesn't constitute a crash and they see atoi returning 0 as legitimate behavior. This is a case where testing with bad data isn't terribly instructive. Yet another reason not to use atoi.

[jephthah]

hey i agree. i rarely use atoi, favoring strtol instead. and i always error check user inputs in my own code.

i am merely answering his simple question: "why doesnt X work?" with a simple answer: "because you can't use X like that... you must use Y instead."

now if you want to diverge into more advanced topics, and give him rope to hang himself... thats fine too. because if he tries to pass that code off in an assignment, and then gets called on to explain it -- and can't -- well, that's probably karma working like it should.

i mean, maybe he'll successfully study and understand what and why you're doing what you're doing, and learn some really valuable lessons. but, i think, just in my brief experience here with other similar posters, that is unlikely.

whatever. its all good. you're right, and you've already won. I'm scared of your chainsaw, okay? I'm just offering the answer at the same level of the question as it was posed. that's all.

[Narue]

>i rarely use atoi, favoring strtol instead. and i always
>error check user inputs in my own code.
I couldn't care less about your own code. You can use every bad habit in the book for your private programs, but the around here, we set a good example for programmers that usually don't know any better.

>why doesnt "x" work? with a simple answer: because
>you can't use "x" like that... you must use "y" instead.
In my experience, they're going to take your word as the word of God and use "y". If you're giving an example, make sure it's a good example, because it's a safe bet that someone will use it in a serious program.

Rather than give a specific solution (especially a bad one), explain why "x" doesn't work and the process that leads to "y". Then you can list standard functions that do "y". For example, "use atoi" is bad, but "you have to convert the string to an integer, and here are a few functions that do it for you...".

>you've already won.
I'm not here to win, but as long as you keep encouraging bad habits, I'll continue to feel like I've lost.

[MaestroRage]

I do appreciate all the input given into this problem.

Jephthah's solution was easy to understand, and provided itself as a stepping stone for me to look into the other given suggestions and assistance. Though I agree that as a beginner some of the input given was a tad complicated, as a whole I think having had both read a simple solution as well as an advanced one has really given me a boost in my understanding as well as my arsenal of tools to consider in future projects.

A sincere thanks to all of you, i'm quite grateful.

[jephthah]

Narue, think about it.... really. its a vending machine problem. CSC 101.

do you think his class is going to be expected to understand strtol pointers and how to use the "errno" library?? He's here because he cant figure out how to cast a char * string into an int.

can you even remember learning the basics how to program? you're an expert among professionals, girlfriend.

these people just want their programs to "work". they cant see the larger picture yet. thats why they're here.

im not saying though, that you shouldnt be giving the complex answers.... it is good, as the OP just stated, to see more options. I think its important that you're here to show them (and me!) the "correct" way to bulletproof code.

im just offering the simple answer for simple questions. for clarity.

[Narue]

>can you even remember learning the basics how to program?
I remember my struggles quite clearly, thank you.

>im just offering the simple answer for simple questions. for clarity.
Fine. Do whatever you want. I'm tired of fighting the same losing battle with every yahoo who thinks he's helping because he makes people happy in the short term.

[jephthah]

Yahooooo!

:P

:(

[lil_panda]

This: value=(int)argv[1]; will give you the ASCII value of argv[1]

[jephthah]

Oh, Spring battles, ye of great import

how petty doth thou parries and thrusts

appear from the height of Midsummer

.