CODEWITHSUNDEEP
csegmentation-fault
vonlolzor
vonlolzor

Reputation: 9

Reading and writing a file using string buffers

I am trying to read and write a text document and then perform operations on it. 

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

int WRITE(char *FILENAME, char *DATA)
{
    FILE *ptr_file;
    ptr_file =fopen(FILENAME, "w");
    if (!ptr_file)
        return 1;
    fprintf(ptr_file,"%s", DATA);
    fclose(ptr_file);
    return  0;
}

char READ(char *FILENAME)
{
    FILE *ptr_file;
    char buf[1000];
    char* ret="";
    ptr_file =fopen(FILENAME,"r");
    if (!ptr_file)
        return "FAIL\n";
    while (fgets(buf,1000, ptr_file)!=NULL)
        ret=strcat("%s",ret);
    fclose(ptr_file);
    return ret;
}

int main()
{   
    char* DAT = "lol";
    char* FILENAME = "output.txt";
    char* NEWDAT;
    int count=0;
    int max=10;

    while (count<max) {
        NEWDAT=READ(FILENAME);
        WRITE(FILENAME,strcat(DAT,NEWDAT));
        count++;
    }
    READ(FILENAME);
    return 0;
}

This is what the compiler says

gcc -Wall -o "filewrite" "filewrite.c" (in directory: /home/x/Documents/programming/C code)
filewrite.c: In function ‘READ’:
filewrite.c:22:3: warning: return makes integer from pointer without a cast [enabled by default]
filewrite.c:26:2: warning: return makes integer from pointer without a cast [enabled by default]
filewrite.c: In function ‘main’:
filewrite.c:38:9: warning: assignment makes pointer from integer without a cast [enabled by default]
Compilation finished successfully.

Then it throws me a Code 139/Segmentation Fault. I... seriously cannot begin to understand what's going on here; at the level i am at.

#

POST-ANSWERS EDIT: FIXED CODE WORKING

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

int WRITE(char *FILENAME, char *DATA)
{
    FILE *ptr_file;
    ptr_file =fopen(FILENAME, "w");
    if (!ptr_file)
        return 1;
    fprintf(ptr_file,"%s", DATA);
    fclose(ptr_file);
    return  0;
}

char* READ(char *FILENAME)
{
    FILE *ptr_file;
    char buf[1000];     
    char *ret = malloc(1); 
    int retsize = 1; 
    ret[0]='\0';
    buf[0]='\0';

    ptr_file = fopen(FILENAME,"r");
    if (!ptr_file) {
        ret = realloc(ret, strlen("FAIL\n") + 1);
        strcpy(ret, "FAIL\n");
        return ret;
    }
    while (fgets(buf,1000, ptr_file)!=NULL)
    {
        retsize += strlen(buf)+1;  // new size is old size + length of string in buf
        ret = realloc(ret, retsize);  // increase the size of the allocation
        strcat(ret, buf);          // append the new data
    }
    fclose(ptr_file);
    return ret;
}

int main()
{   
    char* DAT = malloc(1);
    int datsize = 1;
    char* FILENAME = "output.txt";
    char* NEWDAT; 
    strcpy(DAT, "LOL");
    int count=0;
    int max=5;

    while (count<max) {
        NEWDAT=READ(FILENAME);
        datsize += strlen(NEWDAT) + strlen(DAT);
        DAT = realloc(DAT, datsize);
        strcat(DAT,NEWDAT);
        WRITE(FILENAME,DAT);

        printf("%i\n",count);
        printf("%s\n",DAT); 
        count++;
    }
    READ(FILENAME);
    return 0;
}

Upvotes: 1

Views: 3059

Answers (2)

luser droog
luser droog

Reputation: 19504

This is the big problem I see.

char* ret="";
...
    ret=strcat("%s",ret);

For one, strcat doesn't take a format specification (maybe you're thinking of sprintf).

For two, you cannot modify the contents of a string literal (like "%s" or ""). You have to allocate space or use a character array like you did with buf.

If you want to return the accumulated contents of buf, try

char *ret = malloc(1);
int retsize = 1;
ret[0] = '\0'; // this line could also be written: `strcpy(ret,"");` or `*ret = 0;`
...
while (fgets(buf,1000, ptr_file)!=NULL)
{
    retsize += strlen(buf)+1;  // new size is old size + length of string in buf
    ret = realloc(ret, retsize);  // increase the size of the allocation
    strcat(ret, buf);          // append the new data
}

To remove the compiler warnings, add

#include <stdlib.h>  // malloc

And, as others have said. The return type of the function should be char * to match the variable whose value is being returned.

Addressing the edited code.

NEWDAT is receiving the data read by READ. So it's DAT that needs to be resizable.

    ...
    char* DAT = malloc(1);
    int datsize = 1;
    char* FILENAME = "output.txt";
    char* NEWDAT; 
    DAT[0]='\0';
    int count=0;
    int max=5;

    while (count<max) {
        NEWDAT=READ(FILENAME);
        datsize += strlen(NEWDAT) + 1;
        DAT = realloc(DAT, datsize);
        strcat(DAT,NEWDAT);
        WRITE(FILENAME,NEWDAT);
        ...

As BrainSteel points out, there is another issue with the READ function, although it's not critical for getting this program to run.

if (!ptr_file)
    return "FAIL\n";

Since one path returns this string literal, and the other path returns malloced data, the calling function won't know whether to call free or not. We can fix this by always returning malloced memory.

if (!ptr_file) {
    ret = realloc(ret, strlen("FAIL\n") + 1);
    strcpy(ret, "FAIL\n");
    return ret;
}

Upvotes: 2

John3136
John3136

Reputation: 29266

READ (horrible naming convention BTW) claims to return a char, but your code is acting like it's char*

The strcat("%s", ret) is bad beacuse the destination ("%s") doesn't have any space to append "ret")

Upvotes: 5

Related Questions