I'm new to C and I'm learning about file descriptors and using the open, write, close functions. What I expect is a printf statement outputting the file descriptor after the open() function, and another printf statement outputting a confirmation of the text being written. What I get instead is:
.\File.exe "this is a test"
[DEBUG] buffer @ 0x00b815c8: 'this is a test'
[DEBUG] datafile @ 0x00b81638: 'C:\Users\____\Documents\Notes'
with a blank space where the further debugging should be. The code block for the section is:
strcpy(buffer, argv[1]); //copy first vector into the buffer
printf("[DEBUG] buffer \t @ 0xx: \'%s\'\n", buffer, buffer); //debug buffer
printf("[DEBUG] datafile @ 0xx: \'%s\'\n", datafile, datafile); //debug datafile
strncat(buffer, "\n", 1); //adds a newline
fd = open(datafile, O_WRONLY|O_CREAT|O_APPEND, S_IRUSR|S_IWUSR); //opens file
if(fd == -1)
{
fatal("in main() while opening file");
}
printf("[DEBUG] file descriptor is %d\n", fd);
if(write(fd, buffer, strlen(buffer)) == -1) //wrting data
{
fatal("in main() while writing buffer to file");
}
if(close(fd) == -1) //closing file
{
fatal("in main() while closing file");
}
printf("Note has been saved.");
I basically copied the code word-for-word from the book im studying from, so how could this not be working?
The issue is the print statements aren't printing anything, and the file descriptor isn't being returned.
Here is the full code:
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <fcntl.h>
#include <sys/stat.h>
void usage(char *pnt, char *opnt) //usage function
{
printf("Usage: %s <data to add to \"%s\">", pnt, opnt);
exit(0);
}
void fatal(char*); //fatal function for errors
void *ec_malloc(unsigned int); //wrapper for malloc error checking
int main(int argc, char *argv[]) //initiates argumemt vector/count variables
{
int fd; //file descriptor
char *buffer, *datafile;
buffer = (char*) ec_malloc(100); //buffer given 100 bytes of ec memory
datafile = (char*) ec_malloc(20); //datafile given 20 bytes of ec memory
strcpy(datafile, "C:\\Users\\____\\Documents\\Notes");
if(argc < 2) //if argument count is less than 2 i.e. no arguments provided
{
usage(argv[0], datafile); //print usage message from usage function
}
strcpy(buffer, argv[1]); //copy first vector into the buffer
printf("[DEBUG] buffer \t @ %p: \'%s\'\n", buffer, buffer); //debug buffer
printf("[DEBUG] datafile @ %p: \'%s\'\n", datafile, datafile); //debug datafile
strncat(buffer, "\n", 1); //adds a newline
fd = open(datafile, O_WRONLY|O_CREAT|O_APPEND, S_IRUSR|S_IWUSR); //opens file
if(fd == -1)
{
fatal("in main() while opening file");
}
printf("[DEBUG] file descriptor is %d\n", fd);
if(write(fd, buffer, strlen(buffer)) == -1) //wrting data
{
fatal("in main() while writing buffer to file");
}
if(close(fd) == -1) //closing file
{
fatal("in main() while closing file");
}
printf("Note has been saved.");
free(buffer);
free(datafile);
}
void fatal(char *message)
{
char error_message[100];
strcpy(error_message, "[!!] Fatal Error ");
strncat(error_message, message, 83);
perror(error_message);
exit(-1);
}
void *ec_malloc(unsigned int size)
{
void *ptr;
ptr = malloc(size);
if(ptr == NULL)
{
fatal("in ec_malloc() on memory allocation");
return ptr;
}
}
EDIT: the issue was fixed. The reason for the bug was that not enough memory was being allocated to the ec_malloc function, meaning text couldn't be saved. I changed the byte value to 100 and the code now works.
CodePudding user response:
I am not sure which compiler you are using, but the one I tried the code with (GCC) says:
main.c:34:5: warning: ‘strncat’ specified bound 1 equals source length [-Wstringop-overflow=]
34 | strncat(buffer, "\n", 1); //adds a newline
| ^~~~~~~~~~~~~~~~~~~~~~~~
In other words, the call to strncat
in your code is highly suspicious. You are trying to append a single line-break character, which has a length of 1, which you pass as the third argument. But strncat
expects the third parameter to be the remaining space in buffer
, not the length of the string to append.
A correct call would look a bit like this:
size_t bufferLength = 100;
char* buffer = malloc(bufferLength);
strncat(buffer, "\n", (bufferLength - strlen(buffer) - strlen("\n") - 1));
In this case, however, you are saved, because strncat
guarantees that the resulting buffer is NUL-terminated, meaning that it always writes one additional byte beyond the specified size.
All of this is complicated, and a common source of bugs. It's easier to simply use snprintf
to build up the entire string at one go:
size_t bufferLength = 100;
char* buffer = malloc(bufferLength);
snprintf(buffer, bufferLength, "%s\n", argv[1]);
Another bug in your code is the ec_malloc
function:
void *ec_malloc(unsigned int size)
{
void *ptr;
ptr = malloc(size);
if(ptr == NULL)
{
fatal("in ec_malloc() on memory allocation");
return ptr;
}
}
See if you can spot it: what happens if ptr
is not NULL
? Well, nothing! The function doesn't return a value in this case; execution just falls off the end.
If you're using GCC (and possibly other compilers) on x86, this code will appear to work fine, because the result of the malloc
function will remain in the proper CPU register to serve as the result of the ec_malloc
function. But the fact that it just happens to work by the magic of circumstance does not make it correct code. It is subject to stop working at any time, and it should be fixed. The function deserves a return value!
Unfortunately, the GCC compiler is unable to detect this mistake, but Clang does:
<source>:64:1: warning: non-void function does not return a value in all control paths [-Wreturn-type]
}
^
The major bug in your code is a buffer overrun. At the top, you allocate only 20 bytes for the datafile
buffer:
datafile = (char*) ec_malloc(20); //datafile given 20 bytes of ec memory
which means it can only store 20 characters. However, you proceed to write in more than 20 characters:
strcpy(datafile, "C:\\Users\\____\\Documents\\Notes");
That string literal is 33 characters, not including the terminating NUL! You need a buffer with at least 50 characters of space to hold all of this. With a buffer that is too small, the strcpy
function call creates a classic "buffer overrun" error, which is undefined behavior that manifests itself as corrupting your program's memory area and thus premature termination.
Again, when I tried compiling and running the code, GCC reported:
malloc(): corrupted top size
because it detected that you had overrun the dynamically-allocated memory (returned by malloc
). It was able to do this because, under the hood, malloc
stores sentinel information after the allocated memory block, and your overwriting of the allocated space had written over its sentinel information.
The whole code is a bit suspect; it was not written by someone who knows C very well, nor was it debugged or reviewed by anyone else.
There is no real need to use dynamic memory allocation here in order to allocate fixed-size buffers. If you're going to use dynamic memory allocation, then allocate the actual amount of space that you need. Otherwise, if you're allocating fixed-size buffers, then just allocate on the stack.
Don't bother with complex string-manipulation functions when you can get away with simply using snprintf
.
And as a bonus tip: when debugging problems, try to reduce the code down as small as you can get it. None of the file I/O stuff was related to this problem, so when I was analyzing this code, I replaced that whole section with:
printf("[DEBUG] file descriptor is %d\n", 42);
Once the rest of the code is working, I can go back and add the real code back to that section, and then test it. (Which I didn't do, because I don't have a file system handy to test this.)