-1

I have a problem with exercise 5-13 of K&R, the goal of the exercise is to make a function tail that does the same as the *nix command, here's my function:

#include <stdio.h>
#include <string.h>
#include "tail.h"

int tail(int n)
{
    char *saved_lines[n];
    for (int i = 0; i < n; i++)
        saved_lines[i] = "\0";
    int line_state[n];
    for (int j = 0; j < n; j++)
        line_state[j] = 0;
    int num_lines = 0, i = 0;
    char line[MAXLINE];
    saved_lines[n - 1] = "\0";
    while (get_line(line, MAXLINE) > 0)
    {
        for (i = 0; i < n - 1; i++)
        {
            strcpy(saved_lines[i], saved_lines[i + 1]);
            line_state[i] = line_state[i + 1];
        }
        strcpy(saved_lines[n - 1], line);
        line_state[n - 1] = 1;
    }
    printf("last %d lines: \n", n);
    for (i = 0; i < n; i++)
        if (line_state[i] == 1)
            printf("%d: %s\n", i, saved_lines[i]);
}

problem is when I run it I get a Segmentation fault (core dumped) error, and running it through Valgrind shows the error comes from the call to strcpy:

==25284== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==25284==  Bad permissions for mapped region at address 0x108E64

...I don't get why, at first strcpy had a problem with the saved_lines[i] pointers being non initialized, fixing that with

for(int i=0;i<n;i++)
    saved_lines[i]="\0";

didn't help...any ideas what could cause this ? thanks in advance!


EDIT: initiated --> initialized

ColdCoffeeMug
  • 168
  • 1
  • 13
  • 2
    The pointers are all initialized to point at a string literal. You need some actual storage space. – Martin James Oct 07 '17 at 12:15
  • 2
    OT: "*initiated*" ---> "initialised" – alk Oct 07 '17 at 12:21
  • Allocate some memory to every word in your char array, they are pointing to string literal. char literals are stored as `read-only` somewhere as a part of your binary. And you should know that the `strcpy` modifies its parameters. This will help you, [About string literals](https://stackoverflow.com/questions/2589949/string-literals-where-do-they-go), [SimilarQuestion](https://stackoverflow.com/questions/11379412/crash-when-handling-char-initd-with-string-literal-but-not-with-malloc) – WhiteSword Oct 07 '17 at 12:28

1 Answers1

3

I believe that you've got many more problems, but one is certainly here:

int tail(int n)
{
  char *saved_lines[n];
  for(int i=0;i<n;i++)
    saved_lines[i]="\0";

You are declaring an (variable-length) array of pointers. Then you make this pointer point to a fixed string somewhere in your data segment. Then you go and try to modify that memory. No, no.

You need to reserve memory for each of your lines. You could go for something like:

  for(int i = 0; i < n; i++)
    saved_lines[i] = calloc(MAXLINE, 1);

But from here there's still a lot to do. Plus you will need to free that memory later.

Costantino Grana
  • 3,132
  • 1
  • 15
  • 35
  • i guess it's not possible to use `strcpy` on the uninitialized `saved_lines[i]` ? – ColdCoffeeMug Oct 07 '17 at 12:22
  • 1
    As @alk pointed out "uninitiated" should be "uninitialized", meaning "not initialized", meaning "they point to random things", or in your case "they point to the same string in memory". So no, you cannot `strcpy` to random places in memory. First allocate memory, then use it. This is the ugly thing of C. Oh, and then free it. I forgot to do it also here in the comments... – Costantino Grana Oct 07 '17 at 12:25
  • 2
    OT: "*ugly*" --> "challenging"! ;-) – alk Oct 07 '17 at 12:47
  • @CostantinoGrana you said there were many problems, could you clarify more what those are ? I modified my code to allocate memory for the `saved_lines[i]`and free it at the end of my program but I still have a `Segmentation fault (core dumped)` at `strcpy`... – ColdCoffeeMug Oct 07 '17 at 12:51
  • @Ryuuzaki_kun Did you try to remove the `saved_lines[n - 1] = "\0";` which has no meaning at all? – Costantino Grana Oct 07 '17 at 13:38