I can't find this segmentation fault

user_4934531 0
Viewed 90

So I'm doing this codewars kata and I'm succeding in every test but in the end the result fails because my code has a segmentation fault and I don't think I know enough about the language to find it! Can someone please halp?

int is_valid_ip(const char *addr) 
{
    char set[] = "1234567890";
    int current;
    int octet_counter = 0;
    char *octet = 0;
    octet = strtok(addr, ".");
    while (octet)
    {
        if (strspn(octet, set) != strlen(octet)) return 0; // checks for spaces
        if (strlen(octet) > 1 && (octet[0]) == '0') return 0; // checks for preceding zeros
        sscanf(octet, "%d", &current);
        if (current < 0 || current > 255) return 0; // checks for range
        octet = strtok(0, ".");
        ++octet_counter;
    }
    if (octet_counter == 4) return 1; // checks for number of octets
    return 0;
};

My code was kind of cleaner but after so much messing around trying to solve this problems it's become this...

c11 c
2 Answers

As strtok() modifies the string to be tokenized and addr is defined as const char * (I assume this is a requirement) you may make a copy of the input string *addr:

char ip[16]; // enought to hold nnn.nnn.nnn.nnn
if(strlen(addr)>15) return 0;
strcpy(ip, addr);

subsequently operate on ip instead of addr

Or... avoid using strtok and parse/scan the string without modifying it.

user_15793270

Here's a dumb solution using sscanf, just to show that it's possible. (Below, there are some better solutions.)

#include <stdio.h>

/* True if its argument represents a decimal number between
 * 0 and 255 without leading zeros. Assumes the characters are
 * all digits and that there are at most three of them.
 */
static int good(const char* octet) {
  switch (octet[0]) {
    case '0': return octet[1] == 0;
    case '1': return 1;
    case '2': return octet[1] == 0 || octet[2] == 0
              || octet[1] < '5'
              || octet[1] == '5' && octet[2] < '6';
    default:  return octet[1] == 0 || octet[2] == 0;
  }
}          

struct OctetSep { char oct[4], sep[2] };
int is_valid_ip(const char *addr) {
  struct OctetSep bits[4];
  /* The last %c conversion is expected to fail. */
  return sscanf(addr, "%3[0-9]%1[.]%3[0-9]%1[.]%3[0-9]%1[.]%3[0-9]%1c",
                      bits[0].oct, bits[0].sep,
                      bits[1].oct, bits[1].sep,
                      bits[2].oct, bits[2].sep,
                      bits[3].oct, bits[3].sep) == 7
         && good(bits[0].oct) && good(bits[1].oct)
         && good(bits[2].oct) && good(bits[3].oct);
}

Of course, the bulk of the work is being done by good, and it would be easy to make it a bit more general:

#include <stddef.h>

/* This idiocy is avoid problems with signed characters */
static int my_isdigit(char c) { return c >= '0' && c <= '9'; }

/* Returns the address of the first unmatched character after a match
 * of an integer with no leading zeros and maximum value 255.
 * This address may be in the middle of the integer, if it is too big.
 * If the string doesn't start with a digit, returns NULL.
 */
static const char* good(const char* octet) {
  switch (octet[0]) {
    case '0': return octet + 1;
    case '1': return   !my_isdigit(octet[1]) ? octet + 1
                     : !my_isdigit(octet[2]) ? octet + 2
                     : octet + 3;
    case '2': return   !my_isdigit(octet[1]) ? octet + 1
                     : !my_isdigit(octet[2]) ? octet + 2
                     : octet[1] < '5' || octet[1] == '5' && octet[2] < '6'
                                             ? octet + 3
                     : octet + 2;
    case '3': case '4': case '5': case '6': case '7': case '8':
    case '9': return !my_isdigit(octet[1]) : octet + 1 ? octet + 2;
    default:  return NULL;
  }
}

int is_valid_ip(const char *addr) {
  for (const char* i = "..."; (addr = good(addr)); ++addr, ++i) {
    if (*addr != *i) break;
    if (!*i) return 1;
  }
  return 0;
}

But probably the simplest code comes from the use of strtol, although you still need a bunch of manual checks. (This solution scales much better to applications where the valid integer range is larger than 256.)

#include <stdlib.h>

/* This idiocy is avoid problems with signed characters */
static int my_isdigit(char c) { return c >= '0' && c <= '9'; }

/* Returns the address of the first character following the integer
 * starting precisely at the supplied address, provided the integer
 * has no leading zeros and a maximum value of 255. Otherwise,
 * returns NULL.
 */
static const char* good(const char* octet) {
  if (*octet == '0') return octet + 1;
  /* This check is necessary because strtol also accepts whitespace and - */
  if (!my_isdigit(*octet)) return NULL;
  char *endptr;
  long value = strtol(octet, &endptr, 10);
  /* We already handled 0, so value==0 must be an error return */
  if (value <= 0 || value > 255) return NULL;
  return endptr;
}

int is_valid_ip(const char *addr) {
  for (const char* i = "..."; (addr = good(addr)); ++addr, ++i) {
    if (*addr != *i) break;
    if (!*i) return 1;
  }
  return 0;
}
user_15662210
Related