CODEWITHSUNDEEP
c
Giuseppe Crinò
Giuseppe Crinò

Reputation: 546

Broken code on K&R?

At page29 of 'The C programming language' (Second edition) by K&R I've read a procedure I think is broken. Since I'm a beginner I would expect I'm wrong though I cannot explain why.

Here's the code:

#include <stdio.h>
#define MAXLINE 1000 // Maximum input line size

int get1line(char line[], int maxline);
void copy(char to[], char from[]);

// Print longest input line
int
main()
{
  int len;                // Current line lenght
  int max;                // Maximum lenght seen so far
  char line[MAXLINE];     // Current input line
  char longest[MAXLINE];  // Longest line saved here

  max = 0;
  while ((len = get1line(line, MAXLINE)) > 0)
    if (len > max) {
      max = len;
      copy(longest, line);
    }

  if (max > 0)            // There was a line to read
    printf("Longest string read is: %s", longest);

  return 0;
}

// `get1line()` : save a line from stdin into `s`, return `lenght`
int
get1line(char s[], int lim)
{
  int c, i;

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

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

  s[i] = '\0';

  return i;
}


// `copy()` : copy `from` into `to`; assuming
// `to` is big enough.
void
copy(char to[], char from[])
{
  int i;

  i = 0;
  while ((to[i] = from[i]) != '\0')
    ++i;
}

My perplexity is: we're using the function get1line and suppose that at the end of the for-loop i is set at lim -1. Then the following if-statement will update i at lim, causing the next instruction (the one which puts the NULL character at the end of the string) to corrupting the stack (since s[lim] is not allocated, in that case).

Is the code broken?

Upvotes: 2

Views: 211

Answers (2)

Gareth Rees
Gareth Rees

Reputation: 65864

Summary: It's not possible to exit the loop with both i == lim-1 and c == '\n', so the case you are worried about never arises.

In detail: We can rewrite the for loop (while preserving its meaning) to make the order of events clear.

i = 0;
for (;;) {
    if (i >= lim-1) break;      /* (1) */
    c = getchar();
    if (c == EOF) break;        /* (2) */
    if (c == '\n') break;       /* (3) */
    s[i] = c;
    ++i;
}

At loop exit (1) it can't be the case that c == '\n' because if that were the case then the loop would have exited at (3) the previous time around.*

At loop exits (2) and (3) it can't be the case that i == lim-1 because if that were the case then the loop would have exited at (1).

* This depends on lim being at least 2, so that there was in fact a previous time around the loop. The program only ever calls get1line with lim equal to MAXLINE, so this is always the case.**

** You could make make the function safe when lim is less than 2 by initializing c to a value other than '\n' before the loop begins. But if you are concerned about this possibility, then you might also want to be concerned about the possibility that lim is INT_MIN, so that lim-1 results in undefined behaviour due to integer overflow.

Upvotes: 4

William Morris
William Morris

Reputation: 3684

The code is wrong if lim == 0 because it uses c uninitialised and adds the \0. It is also wrong if lim == 1 because it uses c uninitialised. Calling the function with lim < 2 is not very useful, but it should not fail like this.

If lim > 1 then the function is OK

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

The loop exits either if i == lim-1 or if c == EOF or if c == '\n'.

  • If the first condition is true (i == lim-1), then the last condition is definitely not true (unless lim < 2, as noted above).

  • If the first condition is false (i < lim-1), then even if the loop exits with c == \n, we know that there is space in the buffer because we know that i < lim-1.

Upvotes: 1

Related Questions