Click here to Skip to main content
14,933,981 members
Please Sign up or sign in to vote.
0.00/5 (No votes)
See more:
The program was not working for input 5r i.e in input when first character is number and remaining next character is any alphabet or negative number. For example when I am giving input as 5r in the output I am getting factorial of 5.

So I tried putting check for strtol unsuccessful conversion :-

if (p == buf || *p != '\0'){ printf("\nInvalid input: not a number\n");}

but I am getting output as Invalid input: not a number for all the input.

I found many similar questions on google. However, they don't resolve my issue. I am not understanding what is wrong with this simple check? How can I successfully detect errors from strtol?

What I have tried:

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

int display();
void fact_fun(int num_fact);

int main()
{
    int num;
    while ((num = display()) >= 0)
        {
        fact_fun(num);
        }
    return 0;
}

int display()
{
    char buf[256];
    char *p;
    long value;


    for (;;)
        {
        printf("Enter number to find factorial or press ENTER KEY to exit: ");

        if (fgets(buf, sizeof buf, stdin) == NULL || *buf == '\n')
            return -1;

        errno = 0;
        value = strtol(buf, &p, 0);

        if (p == buf || *p != '\0')
            {
            printf("Invalid input: not a number\n");
            }
        else
            {
            if (value < 0)
            {
                printf("Invalid input: negative values not allowed\n");
            }
            else if (errno != 0 || value > INT_MAX)
                {
                    printf("Invalid input: value too large for type int\n");
                }
                else
                    {
                        return (int)value;
                    }
            }
        }
}

void fact_fun(int num_fact)
{
    int fact = 1;
    for (int i = 1; i <= num_fact; i++)
        {
        if (fact > INT_MAX / i)
        {
            printf("Invalid input: arithmetic overflow\n");
            return;
        }
        fact = fact * i;
    }
    printf("Factorial of %d is %d\n", num_fact, fact);
}
Posted
Updated 2-Oct-18 22:12pm

<pre lang="c++">
value = strtol(buf, &p, 0);

You have set your number base to zero, so there is no number that will be valid.


As pointed out by Jochen, zero is a valid radix.
   
v2
Comments
CPallini 3-Oct-18 3:44am
   
Good catch. 5.
Jochen Arndt 3-Oct-18 4:11am
   
A base of zero is special and allows input as hex, octal, and decimal:

From the strtol man page:

If base is zero or 16, the string may then include a "0x" prefix, and the number will be read in base 16; otherwise, a zero base is taken as 10 (decimal) unless the next character is '0', in which case it is taken as 8 (octal).
CPallini 3-Oct-18 4:21am
   
Wow, I learnt something.
Richard MacCutchan 3-Oct-18 4:40am
   
Thank you. I looked at the MSDN page but missed that part.
You are using fgets() to read the input from stdin. That function will place the terminating '\n' line feed into the buffer. So even when entering a valid number strtol() will stop at that new line character and not at the end of the string.

The simplest solution is changing the condition:
if (p == buf || *p != '\n')
   
Comments
Vasudha Dixit 3-Oct-18 4:17am
   
My God! How stupid I was! Thanks
Jochen Arndt 3-Oct-18 4:22am
   
You are welcome and thank you for accepting my solution.
Maciej Los 4-Oct-18 16:24pm
   
5ed!
Richard is right that your base is wrong, but ... look at your code
if (p == buf || *p != '\0')
    {
    printf("Invalid input: not a number\n");
    }
p is unassigned, so it will never pass the first part of the test.
Since p is not assigned, *p is also undefined, and will probably start giving you access errors which will crash you application with many compilers.

Even if you set p to the user input in buf the the first part the test will succeed - so you will get the message - and even if that didn't happen, any non blank input by the user will make the second part pass.

So pretty much under all circumstances that code will either give you a message, or crash. You need to step back, think about what you are doing, and try coding that again - because what you have at the moment looks to me like "panic code" instead of "thought out" code!
   
Comments
Jochen Arndt 3-Oct-18 4:17am
   
p is initialised by the strtol() function.
Richard MacCutchan 3-Oct-18 4:41am
   
No, I was wrong, see Jochen's comment to my solution.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

  Print Answers RSS
Top Experts
Last 24hrsThis month



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900