Successful use of strtol() in C

Attempted to use strtol() to parse a string into an integer and to output any appropriate error messages for failed conversions. How did I do? How could/should it be done? Code works as intended if I did not miss anything.

#include 
#include 
#include 
#include 
#include 

#define MAXLENGTH 1024

void retrieveStringInput(char *str, size_t buffersize)
{
    for (;;) {
        if (fgets(str, buffersize, stdin) != NULL) {
            str[strcspn(str, "\n")] = '\0';
            return;
        }
        printf("No input detected please try again...\n");
    }
}

int stringToInt(char *str)
{
    char *end = NULL;
    long number = strtol(str, &end, 10);
    if (!*end && end != str) {
        if (number = INT_MAX) {
            printf("ERROR INPUT IS OUT OF RANGE FOR INTEGER VALUE, %d < number < %d\n", INT_MIN, INT_MAX);
            return 0;
        }
        return (int)number;
    }
    printf("STRING FAIILED TO CONVERT...\n");
    return 0;
}

int main(void)
{
    char number[MAXLENGTH];
    int convertNumber = 0;
    retrieveStringInput(number, sizeof number);

    if (number[0] = '0') {
        printf("Your number is: %d\n", convertNumber);
    }

    if ((convertNumber = stringToInt(number)) != 0)
        printf("YOUR NUMBER IS: %d\n", convertNumber);

    printf("Press any key to continue...\n");
    getch();
    return 0;
}

Compiler errors

First of all, this code does not compile with the (IMO too lenient) command line of :

gcc -Wall -Werror main.c -o strtol

Instead it gives the message that @vnp showed in their answer, that you should change if (number[0] = '0') to either if ( (number[0] = '0') ) or to if (number[0] == '0').

Having said that, the command line I usually use (and the one I put in my Makefiles) is:

-Wall -Wextra -Wfloat-equal -Wundef -Werror -fverbose-asm -Wint-to-pointer-cast -Wshadow -Wpointer-arith -Wcast-align -Wstrict-prototypes -Wcast-qual -Wmissing-prototypes -Wstrict-overflow=5 -Wwrite-strings -Wconversion --pedantic-errors -std=gnu11 -ggdb

With these arguments I get the following from my IDE:

error: conversion to ‘int’ from ‘size_t {aka long unsigned int}’ may alter its value [-Werror=conversion] at line 15 col 24

error: using the result of an assignment as a condition without parentheses [-Werror,-Wparentheses] at line 45 col 9

note: place parentheses around the assignment to silence this warning at line 45 col 19

note: use '==' to turn this assignment into an equality comparison at line 45 col 19

Without fixing these, the program does not work quite correctly:

$ ./strtol
4
Your number is: 0
Press any key to continue...

$ ./strtol
888
Your number is: 0
YOUR NUMBER IS: 88
Press any key to continue...

$ ./strtol
11
Your number is: 0
YOUR NUMBER IS: 1
Press any key to continue...

Upon further inspection, the first character is ignored in the output:

$ ./strtol
623642
Your number is: 0
YOUR NUMBER IS: 23642
Press any key to continue...

$ ./strtol
 435345345
Your number is: 0
YOUR NUMBER IS: 435345345
Press any key to continue...

Upon correcting the errors mentioned by the compiler flags, however, the bugs appear to have disappeared!

$ ./strtol
4545
YOUR NUMBER IS: 4545
Press any key to continue...

$ ./strtol
 90
YOUR NUMBER IS: 90
Press any key to continue...

$ ./strtol
asdasdasdadasdasd
STRING FAIILED TO CONVERT...
Press any key to continue...

$ ./strtol
9999999
YOUR NUMBER IS: 9999999
Press any key to continue...

Let this be a lesson to ye -- listen to the C compiler, for it knows many things :)

API

When I write in C, I try to make my functions applicable to a wide variety of uses because C is verbose, simplistic and lacks polymorphism.

Having said that, I'm not sure how I feel about the interface I find when I read your code. Because the point of this program is hardly an end unto itself, surely this has applications in other projects' #includes.

To this end, it would likely be best and most testable if functions didn't have rather unrelated side effects, like printing, which could be handled by the caller.

That is, however, quite subjective and your use-case may differ entirely.

Organisation / Style

I know you're using Windows because you call getch() without an #include , or perhaps that was a typo and you meant getchar() (from `).

So even though all the functions in windows.h are laughable with their longCamelCaseCOMNames, "idiomatic" C uses snake_case_ndrscrs_and_abbrs, not camelCase.

Moreover, in "idiomatic" modern C:



• #include gives us the much cleaner while ( true ) { ...






• a function purporting to call strtol (string to long int) should have type long, not int






• code uses the bool or ssize_t return type, or writes error to a pointer passed as an argument, to indicate error rather than 0






• this way, I don't have to have two if statements every time I want to use your function -- this is almost as bad as atoi.

• always use braces around if statement bodies -- okay, maybe this is personal preference but I think it misleads me and other people reading your code.

How I would write it

I've been writing a lot of C lately, so here's how I'd reimplement your idea.

Mine writes a lot less STDOUT, but you could make it do more with the information the functions give you:

``
#include
#include
#include
#include
#include
#include

#define MAXLENGTH 1024

bool readln (char** buf, const size_t len);
char conv_str (const char const str, long* out);
void str_chomp (char* const str);

// read a line of STDIN
bool readln (char** buf, const size_t len) {

if ( fgets(*buf, len > INT_MAX ? INT_MAX : (int) len, stdin) != NULL) {
str_chomp(*buf);
return true;
}
return false;

}

// try to convert a string to a number or return the erroneous part
char conv_str (const char const str, long* out) {
char* end = NULL;
long number = strtol(str, &end, 10);

if ( end != str && !(*end) ) {
*out = number;
end = NULL;
}

return end;
}

// cut the last newline or don't
void str_chomp (char* const str) {

if (str && (strchr(str, '\n') != NULL) ) {
str[ strcspn(str, "\n") ] = 0;
}
}

int main(void) {

char in_buf = malloc(sizeof (char) MAXLENGTH);

printf("Enter a number! ");

while ( ! readln(&in_buf, MAXLENGTH) ) {
printf("\nEnte

StackExchange Code Review Q#143257, answer score: 14