CODEWITHSUNDEEP
c
jobrien929
jobrien929

Reputation: 155

Sending a string pointer to a function

I'm taking my first C programming course and I've run into a problem trying to write a function that reads a text file one line at a time. Here's my code:

#define LINELENGTH 81

int getLine(char* line, FILE* file) {

    if (line == NULL) {
        line = malloc(sizeof(char) * LINELENGTH);
    }

    fgets(line, LINELENGTH, file);
    int length = strcspn(line, "\n");
    if (line[length] == '\n') {
        line[length] = '\0';
        line = realloc(line, sizeof(char) * (length + 1));
        return length;
    } else {
        char* addThis = NULL;
        int addedLength = getLine(addThis, file);
        length += addedLength;
        line = realloc(line, sizeof(char) * length);
        strcat(line, addThis);
        free(addThis);
        addThis = NULL;
        return length;
    }
}

int main() {
    FILE *text = fopen("test.txt", "r");
    char* line = NULL;
    getLine(line, text);
    printf("The first line is \"%s\"", line);
    fclose(text);
    free(line);
    return 0;
}

My test input file right now just contains a single line, "test"

When I run the program I get "The first line is "(null)"". Not what I was hoping for. When I step thru the function in the debugger everything appears to be working fine inside getLine. But when the function returns all I have left is null.

Any help is appreciated. Thanks.

Upvotes: 2

Views: 287

Answers (5)

varunl
varunl

Reputation: 20229

Considering you haven't done pointer to pointers.

I would suggest changing the method signature as follows:

char* getLine(int *len, FILE* file);
// instead of assigning the value to line like in your case, return it.

Usage would be:

int main() {
  .
  .
  .
  int length;
  char* line = NULL;
  line = getLine(&len, text);
  .
  .
}

Also, I see that you aren't using the return value i.e. the length of the line in main(). So you could omit the length field completely and have a method definition like this:

char* getLine(FILE* file);

Upvotes: 0

Code Monkey
Code Monkey

Reputation: 319

seems like ur going the long way around, i would do something like this:

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

char line[1000];

int main()
{
FILE *file;
file = fopen("test.txt", "r");

fgets(line, sizeof(line), file);
printf("First line: %s", line);
memset(line, 0, strlen(line));
fclose(file);
return 0;
}

and you could incorporate that into a function if needed. or if u wanted to have it list the whole file you could simply do this:

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

char line[1000];

int main()
{
FILE *file;
file = fopen("test.txt", "r");

while(!feof(file))
{
fgets(line, sizeof(line), file);
printf(line);
memset(line, 0, strlen(line));
}

fclose(file);
return 0;
}

Upvotes: 0

Mark Wilkins
Mark Wilkins

Reputation: 41232

The call to getLine is passing the char* pointer by value. The assignment to line within that function does not cause the allocated pointer to be returned to the caller. The function declaration should maybe be:

int getLine(char** line, FILE* file) {...

And then assign the result to *line. And the call to the function would need to pass the address:

getLine( &line, text );

You could also use a local variable for the usage inside the function and then assign the final result to *line prior to returning. That might make the code a bit simpler to understand. Otherwise, each time line is used, it would be necessary to dereference the pointer and it gets (just my opinion here) a bit messier. So maybe change the parameter in the function definition to getLine( char** retLine, ... ). Then declare a local variable of the form char* line;. Then prior to the return statements, assign it:

*retLine = line;

A very incomplete example:

int getLine( char **retLine, FILE *file ) {    
  *retLine = NULL;  // make sure we don't return garbage if error occurs
  char *line = malloc( ... );
  // do stuff with line, fill it up, etc.
  ...
  // assign the final result to the output param
  *retLine = line;
  return length;
}

This is basically a preference issue. Otherwise, the dereferencing must be done. For example,

*line = malloc(...);
int length = strcspn( *line, ... );

etc.

Upvotes: 2

Dmitri
Dmitri

Reputation: 9375

You need to change your getLine function and the way you call it so that it can modify the pointer you pass it. Right now, you only modify the local copy within the function when you allocate or reallocate the line.

Instead of 

int getLine(char* line, FILE* file) {

you need

int getLine(char **line, FILE *file) {

Then, within getLine, you need to dereference line wherever you use it to work with the pointer it points to, eg:

*line = malloc(sizeof(char) * LINELENGTH);

...and...

(*line)[length] = '\0';

...and similarly for the other uses of line.

If you can get rid of the recursion and strcat you can improve the efficiency of your function as well, though the changes above should get it working.

Upvotes: 0

lostyzd
lostyzd

Reputation: 4523

"line" is a pointer, your getLine function only change the copy value of 'line', it doesn't change "line" pointer it self. For your case, you should try

int main() {
    // ..
    line = malloc(sizeof(char) * LINELENGTH);
    getLine(line, text);
    // ..
}

Upvotes: 1

Related Questions