2

I am writing a program that returns a string from stdin, but i am getting warning that it returns an adress of local wariable. How can i return the string?

thanks in advance

#include <stdio.h>
char* readLine()
{
   int i;
   char input[1024];

   for(i=0;i<1024;i++)
   {
     input[i]=fgetc(stdin);
     if(input[i]=='\n')
      {
         break;
      }
   }
   return input;
}

int main()
{
    printf("%s",readLine());
    return 0;
}
Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
nocturne
  • 627
  • 3
  • 12
  • 39
  • 3
    You are returning a local object from a function whose lifetime is over once the function returns. Dynamically allocate memory or pass a variable which can be returned from the function. – P.P Nov 06 '14 at 12:59
  • is this ok? #include #include void nacitajRiadok(char *input[1024]) { int i; for(i=0;i<1024;i++) { *input[i]=fgetc(stdin); if(*vstup[i]=='\n') { break; } } } int main() { char input[1024]; printf("%s",nacitajRiadok(&input)); return 0; } – nocturne Nov 06 '14 at 13:06
  • You should copy past what the compiler precisely tell you. For you question the error is obvious (returning a variable that does not have the copy semantic). You should always give the most information that you have to help others helping you. – mathk Nov 06 '14 at 13:07
  • the second sollution did not have any warnings so i think its ok – nocturne Nov 06 '14 at 13:13
  • Here a good example on how to do different things with strings and pointers in C https://stackoverflow.com/a/46344713/5842403 – Joniale Sep 22 '17 at 08:56

3 Answers3

5

This should work for you:

You can pass input from main as reference:

#include <stdio.h>

char * readLine(char * input, int length) {

    int i;

    for(i = 0; i < length; i++) {
        input[i] = fgetc(stdin);

    input[length] = '\0';

     if(input[i] == '\n')
         break;
   }

   return input;
}

int main() {

    int length = 1024;
    char input[length+1];


    printf("%s", readLine(input, length));

    return 0;

}
Rizier123
  • 58,877
  • 16
  • 101
  • 156
2

Try to do something like that instead:

#include <stdio.h>
char* readLine()
{
   int i;
   char *input;
   if ((input = malloc(sizeof(char) * 1024)) == NULL)
       return (NULL);

   for(i=0;i<1024;i++)
   {
     input[i]=fgetc(stdin);
     if(input[i]=='\n')
      {
         input[i] = '\0';
         break;
      }
   }
   return input;
}

int main()
{
    char *str;
    if (str = readLine()) != NULL) {
        printf("%s\n", str); 
        free(str);
    }
    return 0;
}

}

Swann
  • 2,413
  • 2
  • 20
  • 28
1

There is nothing wrong here - that is just a WARNING because usually it is a common mistake of new programmers. I used to run into problems with this usage all the time.

The first thing... this "string" is not null-terminated. You'll want to put at the end of that function something like *(input + i) = '\0'; and make either the array size 1025 or the condition i < 1023 (so that the null character isn't assigned beyond the end of the buffer), because at the moment using this array in a function that expects null termination will cause it to possibly continue past the end of the array, resulting in a memory access violation. Alternately, you could use memset(input,0,1024);, just still make sure that the condition is something like i < 1023 so that the standard input you receive doesn't end up writing all the way to the last null character in the array.

The other problem is that this memory is local, as in it "belongs" to this function. And for the usage you have here, it is probably just fine to use the same memory... if you plan to call the function, do something with the result, and then call the function again, do something with the result... But if you want to keep what's given to you by it, you'll have to either (1) copy the string to another buffer that isn't going to be written to again when the function is called in the future, or (2) make the function allocate a new buffer each time it runs, and then be sure to delete that memory when you're done with it. For example, instead of char input [1024]; (which by the way would have the same pointer for the life of the program, so it's not really necessary to return it each time) you could write char* input = malloc(1024); and later, when the caller is done with the string, you should free(input);. (Of course, the name might not be input in this case since you would probably not want to free the memory in the function whose purpose is to allocate it.)


I will edit this later with code showing changes.

rsethc
  • 2,485
  • 2
  • 17
  • 27