Segmentation Fault(core dump)

Question

I am getting Segmentation error(core dump) in the following code:

void Update_Log( )
{
        struct logData update;
        int file;

        char *writeBuffer=NULL;

        if((file=creat("/home/user/Desktop/DMS/DMS/filename.txt",O_RDONLY|O_WRONLY))==-1)
                perror("file not opened");
        update.clientName="user";
        update.filename="user";
        update.timestamp="some time";
        sprintf(writeBuffer,"%s %s %s",update.clientName,update.filename,update.timestamp);


        if((write(file,writeBuffer,sizeof(writeBuffer)))==-1)
                perror("write unsuccessful");
        close(file);
}

My structure is as follows:

struct logData
{
        char *clientName;
        char *filename;
        char *timestamp;
};

Can anyone help in this?

The first thing you should do when getting a crash, segmentation fault or other, is to run your program in a debugger. It will help you find the location of the crash, and also let you examine variables to see what might have caused it. — Some programmer dude
– Some programmer dude, Commented Aug 20, 2012 at 8:47

MByD · Accepted Answer · 2012-08-20 08:46:27Z

2

You're trying to write to writeBuffer which is a null pointer, you should declare it as array(1), or allocate a memory on the heap(2) for it.

char writeBuffer[100];
char *writeBuffer=malloc(100)

in both cases you should not use sprintf, but snprintf to make sure you are not overflowing your buffer.

answered Aug 20, 2012 at 8:46

MByD

138k30 gold badges269 silver badges278 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

bchurchill · Accepted Answer · 2012-08-20 08:47:54Z

0

When 'update' is created it only allocates the space for the pointers of the strings clientName, filename, timestamp; it does not allocate space for the string itself. A quick hack would be to do something like update.clientName = (char*)malloc(sizeof(char)*len) where len is the length of what you want to allocate. Then you should really check the return code of that call...

This is exactly the same issue as with the write buffer... you just need to make sure you allocate space for all your strings.

answered Aug 20, 2012 at 8:47

bchurchill

1,4209 silver badges24 bronze badges

1 Comment

Werner Henze Over a year ago

This is true, but not the reason for the crash.

Some programmer dude · Accepted Answer · 2012-08-20 08:49:12Z

0

The error is these two lines:

char *writeBuffer=NULL;

/* ... */

sprintf(writeBuffer, /* ... */);

Here you write into a NULL pointer.

Create an array instead, e.g.

char writeBuffer[32];

answered Aug 20, 2012 at 8:49

Some programmer dude

411k36 gold badges420 silver badges655 bronze badges

Comments

Eric · Accepted Answer · 2012-08-20 08:50:20Z

0

You need to allocate space for writeBuffer.

You could use malloc, or if you have limits on the size of the data (eg if it will be less than 256 bytes) just declare it as

char writeBuffer[256];

I'd also recommend looking up and using snprintf instead of sprintf to make sure data doesnt run off the end.

answered Aug 20, 2012 at 8:50

Eric

3,18219 silver badges14 bronze badges

Comments

Stephane Rouberol · Accepted Answer · 2012-08-20 08:51:19Z

0

your struct store only pointers, no memory is allocated for the actual data. Either you should have

struct logData
{
        char clientName[SOMEDEFINE1NUMBERHERE];
        char filename[SOMEDEFINE2NUMBERHERE];
        char timestamp[SOMEDEFINE3NUMBERHERE];
};

or you allocate the memory (malloc)

update.clientname = (char *)malloc(SOMEDEFINE1NUMBERHERE); strncpy(update.clientname, "user");

answered Aug 20, 2012 at 8:51

Stephane Rouberol

4,40421 silver badges18 bronze badges

Comments

Werner Henze · Accepted Answer · 2020-05-10 19:18:48Z

As Binyamin Sharet wrote sprintf is not allocating the output buffer. The caller must provide an output buffer that is long enough. This is not always simple. Therefor I advise you to use fopen instead of open. Then you can code like that:

void Update_Log( )  
{  
        struct logData update;  
        FILE *file = fopen("/home/user/Desktop/DMS/DMS/filename.txt", "w");  

        if(file==NULL) {  
                perror("file not opened");  
                return;
        }
        update.clientName="user";  
        update.filename="user";  
        update.timestamp="some time";  
        if(fprintf(file,"%s %s %s",update.clientName,update.filename,update.timestamp) < 0)  
                perror("write unsuccessful");  
        fclose(file);

}

As you can see the code is shorter and it has no potential buffer overflow.

Kiril Kirov · Accepted Answer · 2012-08-20 08:48:56Z

-1

It crashes, because writeBuffer is NULL.

You need to allocate memory for writeBuffer using malloc - use strlen (for example) to calculate the neccesary space (don't forget \0 and the spaces in the sprintf).

OR, you may allocate some fixed size array automatically:

char writeBuffer[ SOME_SIZE_BIG_ENOUGH ];

Also, you should not do this:

    update.clientName="user";
    update.filename="user";
    update.timestamp="some time";

You have several options:

use strcpy
use strdup (POSSIX)
make the pointers const char*, not just char*.

answered Aug 20, 2012 at 8:48

Kiril Kirov

38.5k26 gold badges120 silver badges195 bronze badges

Collectives™ on Stack Overflow

Segmentation Fault(core dump)

7 Answers 7

Comments

1 Comment

Comments

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

7 Answers 7

Comments

1 Comment

Comments

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related