Why is string modified in C even when I'm not trying to modify it?

user_13464086 0
Viewed 77

I'm trying to solve exercise 1-19 in K&R C second edition. "Write a function reverse that reverses the character string s. Use it to write program that reverses its input a line at a time."

My solution takes two input strings s and t. s is source and t is target. And it copies the data in source s to t. I'm able to solve the problem, but I'm struggling hard to understand why would source string s be modified, even though it is not on the left hand side of the equal operator.

#include <stdio.h>

/* Solution to Exercise 1-19. Chapter 1 */

#define MAXLENGTH 10

int getln(char s[], int lim);
void reverse(char s[], char t[]);

int main()
{
  int i, len;

  char s[MAXLENGTH]; /* original string */
  char t[MAXLENGTH]; /* reversed string */

  while ((len = getln(s, MAXLENGTH)) > 0) {
    printf("before reverse: %s", s);
    reverse(s,t);
    printf("reversed string: %s\n", t);
    printf("after reverse: %s", s);
  }  
  return 0; 
}

/* getln: read a line into s, return length */
int getln(char s[], int lim)
{
  int c, i, l; 

  l = 0; 
  for (i = 0; ((c = getchar()) != EOF) && (c != '\n'); ++i) {
    if (i < (lim - 1)) {
      s[l] = c; 
      ++l;
    }  
  }  

  if (c == '\n') {
    s[l] = c; 
    ++l;
  }  

  s[l] = '\0';
  return l; 
}

/* reverse: reverses s to target t */
void reverse(char s[], char t[])
{
  int i, j; 
  for (i = 0; s[i] != '\0'; ++i)
   ;  
  --i;                                                                                                                                                                             
  if (s[i] == '\n') {
    --i;
  }  
  for (j = 0; i >= 0; ++j) {
    t[j] = s[i];
    --i;
  }  
  t[j] = '\0';
}

Test case:

$ ./a.out < testdata 
before reverse: abcdefghi
reversed string: ihgfedcba
after reverse: abcdefghi
ihgfedcba$

Contents of file testdata:

$ cat testdata 
abcdefghijklmnopqrstuvwxyz
$
function-definition c-strings kernighan-and-ritchie reverse c
2 Answers

There is a bug in the function getln To simplify the analyze of the function let's assume that lim is equal to 2.

Then in this loop

  l = 0; 
  for (i = 0; ((c = getchar()) != EOF) && (c != '\n'); ++i) {
    if (i < (lim - 1)) {
      s[l] = c; 
      ++l;
    }  
  }

you can write lim-1 characters that is only one character. The loop stops its iterations when the user will press the key Enter that sends to the input buffer the new line character '\n'.

So the last read character is the new line character '\n'. This character is stored in the string after the loop

  if (c == '\n') {
    s[l] = c; 
    ++l;
  }

Now the limit is exhausted. Two characters of the passed character array are set.

However in the next statement

  s[l] = '\0';

there is access to the memory beyond the limit when l is equal to 2.

That is all. The function invokes undefined behavior provided that the value of the parameter lim is equal to the size of the passed character array. The terminating zero character '\0' is written in the memory outside the character array and later can be overwritten.

I would define the function the following way as it is shown in the demonstrative program below.

#include <stdio.h>

size_t getln( char s[], size_t n )
{
    size_t i = 0;

    if ( n )
    {
        int c;

        while ( i + 1 < n && ( c = getchar() ) != EOF && c != '\n' )
        {
            s[i++] = c;
        }

        if ( c == '\n' && i + 1 < n ) s[i++] = c;

        s[i] = '\0';
    }       

    return i;   
}

int main(void) 
{
    enum { N = 10 };
    char s[N];

    while ( getln( s, N ) ) printf( "\"%s\"\n", s );

    return 0;
}

If to enter

abcdefghijklmnopqrstuvwxyz

then the program output will be

"abcdefghi"
"jklmnopqr"
"stuvwxyz
"

That is only the last entered string contains the new line characters.

Pay attention to that in the exercise there is written

Write a function reverse that reverses the character string s.

This means that you need to reverse the original string itself instead of coping it in the reverse order to another character array.

Such a function can look the following way

#include <stdio.h>

char * reverse( char *s )
{
    size_t n = 0;

    while ( s[n] != '\0' ) n++;

    if ( n && s[n-1] == '\n' ) --n;

    for ( size_t i = 0; i < n / 2; i++ )
    {
        char c = s[i];
        s[i] = s[n-i-1];
        s[n-i-1] = c;
    }

    return s;
}

size_t getln( char s[], size_t n )
{
    size_t i = 0;

    if ( n )
    {
        int c;

        while ( i + 1 < n && ( c = getchar() ) != EOF && c != '\n' )
        {
            s[i++] = c;
        }

        if ( c == '\n' && i + 1 < n ) s[i++] = c;

        s[i] = '\0';
    }       

    return i;   
}

int main(void) 
{
    enum { N = 10 };
    char s[N];

    while ( getln( s, N ) ) printf( "\"%s\"\n", reverse( s ) );

    return 0;
}

Again if the input is

abcdefghijklmnopqrstuvwxyz

then the program output is

"ihgfedcba"
"rqponmlkj"
"zyxwvuts
"

If you want to remove the new line character '\n' from the string inside the function reverse then substitute this statement

    if ( n && s[n-1] == '\n' ) --n;

for this one

    if ( n && s[n-1] == '\n' ) s[--n] = '\0';
user_28772410

You are not allocating memory for c or t, therefore you are overwriting things.

user_29772560
Related