Thread: How to fix a string overflow?

Is this the do/while you have questions about?

Code:

  do {
  printf("Enter two letters separated by hyphens: ");
  if(fgets(s, MAXSIZE, stdin))
   printf("You entered: %s", s);
  } while((!isalpha( (unsigned char) s[0]) ) && (s[1] != '-') &&
           (!isalpha( (unsigned char) s[2]) ) );

But either way why are you casting the character to an unsigned character?

Jim

> So, what's the definitive way to fix this problem?
There isn't one definitive way to fix it, but probably the most common involves putting a check to ensure you don't exceed the bounds in any loop you use on that string. Perhaps:

Code:

while (i < MAXLINE && s[i] != '\0')  // while i is within bounds, and we haven't hit the null terminator
// similar version with for loop
for (i = 0; i < MAXLINE && s[i] != '\0'; i++)

Or, alternatively, you can put an if statement in the loop that breaks out when the limit is reached:

Code:

while (s[i] != '\0') {
    if (i < MAXLINE) {
        break;
    }
}

I usually prefer putting a fundamental condition like that in the loop condition itself, but sometimes, if your logic for incrementing i is a little complicated, the if/break solution ends up cleaner.

There is no need to cast the char to an unsigned char. Just use the char.

Jim

Originally Posted by jimblumberg

There is no need to cast the char to an unsigned char [when using the is* or to* functions]. Just use the char.

Is there is a chance the value falls outside the range of unsigned char, the cast prevents Undefined Behaviour.
The range of a char (on some implementations) includes negative numbers: the range of unsigned char never includes negative numbers.

Originally Posted by jimblumberg

There is no need to cast the char to an unsigned char. Just use the char.

n1570.pdf: 7.4.1:

In all cases the argument is an int, the value of which shall be
representable as an unsigned char or shall equal the value of the macro EOF. If the
argument has any other value, the behavior is undefined.

If you're using an implementation where char is signed it's likely they'll be passing a negative non-EOF value to those functions, which yields undefined behaviour according to that above quotation. It's necessary to cast the char value to an unsigned char before you cast it to an int in such a scenario.

Code:

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

#ifndef ALPHA
   #define ALPHA 26
#endif   

#ifndef MAXSIZE 
   #define MAXSIZE 20
#endif   

static int nLetters;

int justLetters(char*);
void expand(char*, char*);
void shellsort(char*, int);

void shellsort(char* a, int n) 
{ 
   int gap, i, j, temp;
   
   for(gap = nLetters/2; gap > 0; gap /= 2) 
    for(i = gap; i < nLetters; i++)     
      for(j = i - gap; j >= 0 && a[j] > a[j + gap]; j -= gap) 
      { 
         temp = a[j];
         a[j] = a[j + gap];
         a[j + gap] = temp; 
      }       
}     


int justLetters(char* s)
{ 
  int i, j;
  
  for(i = 0, j = 0; i < strlen(s); i++) 
  { 
    if(isalpha( (unsigned char) s[i]) )
    { 
       s[j++] = s[i];        
       nLetters++;
    }    
  }
  s[j] = '\0';            
  return nLetters;
}     

void expand(char* s, char* result) 
{ 
   int c;
   nLetters = justLetters(s);
   shellsort(s, nLetters);
   printf("The expanded string is ");
   for(c = s[0]; c <= s[nLetters - 1]; c++) 
     putchar(c);
   putchar('\n');  
}     

int main(void) 
{ 
  char *s = malloc(MAXSIZE * sizeof(char));    
  char *result = malloc(MAXSIZE * sizeof(char));
  
  do { 
    printf("Enter n letters separated by hyphens: ");
    fgets(s, MAXSIZE, stdin);
   } while(strlen(s) > MAXSIZE); 
   
  printf("You entered: %s\n", s);
  expand(s, result); 
  
  free(s);
  free(result);
  return 0;
}

Is it just my impression or the do-while limited the input to the MAXSIZE I coded instead of obliging me to input again?

Code:

 
./expand
Enter n letters separated by hyphens: a-c-d-b-q-t-y-u-x-d-e-f-f-q-a-b-c-h-j-k-l-paaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
You entered: a-c-d-b-q-t-y-u-x-d
The expanded string is abcdefghijklmnopqrstuvwxy

The only time a value will fall outside the range of a char will be if you use actually use an int for the parameter. Since you are passing a char, there is no way you will have problems since the function is defined as int isalpha(int), therefore the cast is unnecessary in this instance.

Jim

Originally Posted by jimblumberg

The only time a value will fall outside the range of a char will be if you use actually use an int for the parameter. Since you are passing a char, there is no way you will have problems since the function is defined as int isalpha(int), therefore the cast is unnecessary in this instance.

Jim

I think you missed the part where the value of the int "shall be representable as an unsigned char". A char might contain a negative value on implementations where char is signed, and that does not fulfill the requirement.

I fixed the program rewriting the for's:

Code:

 
for(c = letters[0]; c <= letters[nLetters - 1]; c++) 
     putchar(c);
   for(c = digits[0]; c <= digits[nDigits - 1]; c++)
     putchar(c);

but I still don't understand why the other for doesn't work. because I thought

Code:

 
c <= letters[nLetters - 1] 
c < letters[nLetters]

were semantically and logically equivalents. Weren't they? I'm really puzzled:

considering nLetters equals to 3:

Code:

 
c <= letters[2]  -  0 1 2
c < letters[3]    -  0 1 2

It depends on the data of the char.

Maybe you mean this

Code:

#include <string.h>

int main(void)
{
    int array[5]={0,1,2,3,4};
    int i;  

    for( i = 0 ; i < 5 ; i++ )
        printf("%d\n",array[i];

    for( i = 0 ; i <= 4 ; i++ )
        printf("%d\n",array[i];
   
    return 0;
}

Code:

 It depends on the data of the char

how come? why didn't it work for my example?