Reputation: 824
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?
Upvotes: 0
Views: 3130
Reputation: 16781
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.
Upvotes: 0
Reputation: 4384
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");
Upvotes: 0
Reputation: 3172
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.
Upvotes: 0
Reputation: 409482
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];
Upvotes: 0
Reputation: 38193
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:
strcpy
strdup
(POSSIX)const char*
, not just char*
.Upvotes: -1
Reputation: 1420
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.
Upvotes: 0
Reputation: 137442
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.
Upvotes: 2