Pointer char function not running the second time

Question

I have written this function which reads a file, compares a string to each line and if the line contains the string, it return it.

    char* FileSearch2 (char* File_Name , char* String) { 
        FILE * T = fopen(File_Name, "rt");
        char* Line = (char*) malloc (sizeof(char*) * 1024);
        strcat(String,"\t");
        if (T) {
            while (fgets(Line,1024,T)) {
                if (strstr(Line, String) != NULL) {
                    fclose(T);
                    return Line;
                }
            }
        }
        fclose(T);
        return "0";
    }

The problem is that, the second time I run this function it always returns "0".

For example

char* FirstRun = FileSearch2 ("File.txt", Value); // Assuming the value is "Hello", it returns the line

Now

char* SecondtRun = FileSearch2 ("File.txt", Value); // Assuming the value is "Hello", it returns "0"

I would like to know what exactly I am doing wrong.

Maybe unrelated questions: 1) why you allocate 128 characters buffer but you ask to read up to 1024? 2) Why you return a _static_ constant to caller function??? Will they free that memory like the other case? 3) Why you cast malloc return value? 4) Why do you change a parameter? Especially if it's a constant like in your example...what will happen to memory??? — Adriano Repetti, Nov 18 '13 at 13:56
Please, free your memory when done with it too. You are requesting lots of 1024 char chunks that never seem to be released. — The Marlboro Man, Nov 18 '13 at 13:58

score 3 · Accepted Answer · edited May 23 '17 at 10:31

3

Your code has a few serious errors.

For instance, your malloc() logic is all wrong.

You seem to want to allocate room for a line of 1024 characters, but you do this:

char* Line = (char*) malloc (sizeof(char*) * 128);

it should just be:

char *Line = malloc(1024);

Don't cast the return value of malloc() in C, and the business with sizeof(char *) is just plain wrong and confused.

Then you fail to free() the line if the line is not found, which is another problem.

Your code also changes the caller's data by calling strcat(), which means that if you call it with e.g. a string literal (as your example implies) it's totally undefined behavior what's going to happen. You should re-think that part with the '\t'-appending, it's not the best way to do it.

edited May 23 '17 at 10:31

Community

1
1

answered Nov 18 '13 at 13:57

unwind

391,730
64
469
606

It's fine on a 64-bit system, (well, not really:). – Martin James Nov 18 '13 at 13:58
Thank you unwind, I think I understood what's the problem, I'll accept your answer – Transcendent Nov 18 '13 at 14:01
This answer is useful, but it's unlikely to solve the problem as reported. See my answer for a hint on that! – Roddy Nov 18 '13 at 14:05

Roddy · Answer 2 · 2013-11-18T14:03:15.487

2

Well, I expect it's because you function FileSearch modifies String by appending a tab character to it. So the second time through, you're looking for a string with two tabs at the end.

You should make a copy of String, append the tab to the copy, and free the copy at the end.

char *copy = strdup(String);

strcat(copy,"\t");
if (T) {
    while (fgets(Line,1024,T)) {
        if (strstr(Line, copy) != NULL) {
            fclose(T);
            free(copy);
            return Line;

And, as @unwind says, you need to free Line (and copy) if the string isn't found, too.

edited Nov 18 '13 at 14:03

answered Nov 18 '13 at 13:57

Roddy

66,617
42
165
277

Thank you very much Roddy indeed – Transcendent Nov 18 '13 at 14:32

Pointer char function not running the second time

2 Answers2