-1

I'm trying to make a function to validate mobile entry, the mobile number MUST starts with 0 and is 11 numbers (01281220427 for example.)

I want to make sure that the program gets the right entry.

This is my attempt:

#include <stdio.h>
#include <strings.h>


void integerValidation(char x[15]);

int main(int argc, char **argv)
{
    char mobile[15];
    integerValidation(mobile);
    printf("%s\n\n\n", mobile);
    return 0;
}


void integerValidation(char x[15]){
    char input[15];
    long int num = -1;
    char *cp, ch;
    int n;
    printf("Please enter a valid mobile number:");
    while(num<0){
        cp = fgets(input, sizeof(input), stdin);
        if (cp == input) {
        n = sscanf(input, "%ld %c", &num, &ch);
        if (n!=1) {printf("ERROR! Please enter a valid mobile number:");
            num = -1;
        }
        else if (num<0)
            printf("ERROR! Please enter a valid mobile number:");
        else if ((strlen(input)-1)>11 || (strlen(input)-1)<11 || strncmp(&input[0], "0", 1) != 0){
            printf("ERROR! Please enter a valid mobile number:");
            num = -1;
        }
        }
    }

    long int i;
    i = strlen(input);
    //Because when I try to print it out it prints a line after number.
    strcpy(&input[i-1], "");
    strcpy(x, input);

}

Now, if I don't use

strcpy(&input[i-1], "");

the array prints a new line after the number, what would be a good fix other than mine? and how can I make this function optimized and shorter?

Thanks in advance!

Edit: My question is: 1. Why does the input array prints a new line in the end? 2. How can I make this code shorter? End of edit.

  • Well first, be careful when using functions like ```strcpy()``` or ```gets()``` that can result in buffer overflows which can cause security issues especially during validation. It's not really clear though what the issue is nor where you're having it. Try to be more specific in the post. – loremIpsum1771 Dec 26 '15 at 22:51
  • "*Why does the input array prints a new line in the end?*" because `fgets()` is *specified* to hold the final new-line given to enter the string, if not done by an `EOF`. – alk Dec 26 '15 at 23:05
  • If the number begins with `0` then `sscanf(input, "%ld %c", &num, &ch);` will ignore the leading zero. Better to stay with strings and test each char with `isdigit()`. – Weather Vane Dec 26 '15 at 23:15
  • `if (n!=1)` - but you have requested 2 conversions from `sscanf(input, "%ld %c", &num, &ch);`. – Weather Vane Dec 26 '15 at 23:16

3 Answers3

1

If you insist on using sscanf(), you should change the format this way:

int integerValidation(char x[15]) {
    char input[15], c;

    printf("Please enter a valid mobile number:");
    while (fgets(input, sizeof(input), stdin)) {
        if (sscanf(input, "%11[0123456789]%c", x, &c) == 2
         && x[0] == '0' && strlen(x) == 11 && c == '\n') {
            // number stored in `x` is correct
            return 1;
        }
        printf("ERROR! Please enter a valid mobile number:");
    }
    x[0] = '\0';  // no number was input, end of file reached
    return 0;
}

%12[0123456789] parses at most 11 characters that must be digits. %c reads the following character, which should be the trailing '\n'. I verify that both formats have been matched, and the number starts with 0 (x[0] == '0') and it has exactly 11 digits.

chqrlie
  • 131,814
  • 10
  • 121
  • 189
0

You're seeing the newline, since fgets() reads until an EOF or a newline is received. The newline is stored in the buffer, and after that the string is terminated with '\0'.

An alternative would be to directly overwrite the newline with another null-byte: input[i-1] = '\0' (which basically does the same thing as your solution, but saves a function call).

The same goes for the check with strncmp with length 1, you can directly check input[0] == '0'. Note that you have to compare against '0' (char) here, not "0" (string).

A few other things I'm seeing:

You can also spare the %c in the format string for sscanf (you're never evaluating it anyway, since you're checking for 1 as return value), which also eliminates the need for char ch.

Also, you're passing char x[15] as argument to your function. This is a bit misleading, because what actually gets passed is a pointer to a char array (try using sizeof(x), your compiler will most likely issue a warning about the size of char * being returned by sizeof()).

What you could do is to ditch the char array input, which you're using as temporary buffer, and use the buffer which was handed over as argument. For this to be save, you should use a second funcion parameter to specify the size of the buffer which was handed to the function, which would result in a function header like as follows:

void integerValidation(char *input, size_t len);

With this, you'd have to use len instead of sizeof(input). The following question provides more detail why: C: differences between char pointer and array Since you're not using a temporary buffer anymore, you can remove the final call to strcpy().

There are also a lot of checks for the number length/format. You can save a few: If you use %lu instead of %ld no signed numbers are being converted, which saves you the check for num < 0. You're checking whether the length of the read number is <11 or >11 - why not just check for !=11?

You're calling strlen() three times on the input-buffer (or still twice with the reworked check for lengh 11) - it makes sense to call it once, save the length in a variable and use that variable from then on, since you're not altering the string between the calls.

Community
  • 1
  • 1
andreas-hofmann
  • 2,378
  • 11
  • 15
0

There is already an accepted answer, but for what it's worth, here is another.

I made several changes to your code, firstly avoiding "magic numbers" by defining the phone number length and an arbitrarily greater string length. Then there is no point passing an array x[15] to a function since it pays no regard to its length, might as well use the simpler *x pointer. Next, I return all reasons for failure back to the caller, that's simpler. And instead of trying to treat the phone number as a numeric entry (note: letters, spaces, hyphens, commas and # can sometimes be a part of phone number too) I stick to a character string. Another reason is that the required leading zero will vanish if you convert the entry to an int of some size. I remove the trailing newline that fgets() reads with the input line, and the result is this.

#include <stdio.h>
#include <string.h>
#include <ctype.h>

#define MAXLEN 11
#define STRLEN (MAXLEN+10)

int integerValidation(char *x);

int main(int argc, char **argv)
{
    char mobile[STRLEN];
    while (!integerValidation(mobile))      // keep trying
        printf("Invalid phone number\n");
    printf("%s\n\n\n", mobile);             // result
    return 0;
}

int integerValidation(char *x)
{
    int i, len;
    printf("Please enter a valid mobile number:");
    if(fgets(x, STRLEN, stdin) == NULL)     // check bad entry
        return 0;
    x [ strcspn(x, "\r\n") ] = 0;           // remove trailing newline etc
    if((len = strlen(x)) != MAXLEN)         // check length
        return 0;
    if(x[0] != '0')                         // check leading 0
        return 0;
    for(i=1; i<len; i++)                    // check all other chars are numbers
        if(!isdigit(x[i]))
            return 0;
    return 1;                               // success
}
Weather Vane
  • 33,872
  • 7
  • 36
  • 56