*pointer_variable != '\0' is not working for the check of unsuccessful conversion in strtol() function

Question

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 in Stack Overflow. 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?

#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("\nEnter 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("\nInvalid input: not a number\n");
            }
        else
            {
            if (value < 0)
            {
                printf("\nInvalid input: negative values not allowed\n");
            }
            else if (errno != 0 || value > INT_MAX)
                {
                    printf("\nInvalid 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("\nInvalid input: arithmetic overflow\n");
            return;
        }
        fact = fact * i;
    }
    printf("\nFactorial of %d is %d\n", num_fact, fact);
}

As the documentation says, if the string is *not* entirely a number then the pointer is modified. But if it is a number it’s not modified. So it is not pointing at the null in the end — Sami Kuhmonen, Oct 03 '18 at 05:18
@SamiKuhmonen that is what I am asking ,my string is `5r`, it is not entirely number then the pointer should get modified but it is not. — Vasudha Dixit, Oct 03 '18 at 05:29
Your question states the check fails for strings that are numbers and that’s the reason why. If that’s not the case, please edit the question and clarify. — Sami Kuhmonen, Oct 03 '18 at 05:31
Note that you could type blanks before, or after, a valid number. Do you want to count those? As the first answer correctly identifies, your problem is that the string from `fgets()` contains `"5r\n"`, and the newline is not the same as a null byte. — Jonathan Leffler, Oct 03 '18 at 05:41
See also [Correct usage of `strtol()`](https://stackoverflow.com/questions/14176123/correct-usage-of-strtol). — Jonathan Leffler, Oct 03 '18 at 05:46

Fuel · Accepted Answer · 2018-10-03T14:32:36.363

2

The string you get from fgets contains '\n' as last char because you hit enter, so replace it with '\0'. That is a common error we C coders sometimes make.

Edit: So I have tested it myself, and you're right, the reason is that strtoI does not mess with line terminator, so now it works fine with the following check:

*p != '\n'

The full working code is this:

#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("\nEnter 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 != '\n')
            {
            printf("\nInvalid input: not a number\n");
            }
        else
            {
            if (value < 0)
            {
                printf("\nInvalid input: negative values not allowed\n");
            }
            else if (errno != 0 || value > INT_MAX)
                {
                    printf("\nInvalid 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("\nInvalid input: arithmetic overflow\n");
            return;
        }
        fact = fact * i;
    }
    printf("\nFactorial of %d is %d\n", num_fact, fact);
}

edited Oct 03 '18 at 14:32

answered Oct 03 '18 at 05:34

Fuel

157
6

It is often effective to use `buf[strcspn(buf, "\n")] = '\0';` to zap the newline (if there is one). This is safe as written even if there is no newline before the null byte that marks the end of the string (the null byte is overwritten with another). This avoids a variety of problems with `strlen()` and so on. – Jonathan Leffler Oct 03 '18 at 05:43
I tried replacing `\n` with `\0` but this doesn't resolve the issue. Getting output as `Invalid input: not a number` for all the input(for number also) – Vasudha Dixit Oct 03 '18 at 05:46
I mean why cheking p==buf? – Fuel Oct 03 '18 at 05:51
@EugenioUllauri I don't see any expression in my code that you have mentioned – Vasudha Dixit Oct 03 '18 at 05:55
The line after strtoI, – Fuel Oct 03 '18 at 05:58
Try strtol(buf, &p, 10) – Fuel Oct 03 '18 at 06:00
@VasudhaDixit: When I use `value = strtol(buf, &p, 0); if (p == buf || (*p != '\n' && *p != '\0'))` — a minor modification to your code — the program works correctly. Alternatively, if you add `buf[strcspn(buf, "\n")] = '\0';` immediately after the `if` block calling `fgets()` (and `#include ` at the top), your code works. If neither of these works for you, then you've probably made a transcription error. Note that it is conventional in C to use `else if` when possible, and to align `else if` with the `if`. Your code nests statements unnecessarily. – Jonathan Leffler Oct 03 '18 at 14:27

*pointer_variable != '\0' is not working for the check of unsuccessful conversion in strtol() function

1 Answers1