Ad

Is Repetitively Creating Buffers On The Stack In A Loop In C Bad Practice?

The title of this post is very similar to what I searched in relation to this. Every result I came across was about buffer overflows, and that isn't what I'm after.

I have my function iterating through each filename in a dirent structure I've previously populated. Each filename varies in size, from being very small, to being quite large.

Previously what my function would do, is create the buffer, with a size of 2048 bytes. Then enter the loop. During each iteration of the loop, the buffer is filled with a path to the target directory, plus the current filename within the directory concatenated onto the end of it. With the new path within the buffer, I perform some fairly small file operations. This happens until the final filename in the structure has been reached.

The problem however is that not every full path is going to be 2048 bytes. Some may not even a third of this size.

Revisiting this function, I moved the creation of the buffer to within the loop, and each iteration of the loop creates the buffer, of size n, where n is the length of the target directory + the length of the current filename within the directory.

I am wondering whether or not this may be considered bad practice or anything otherwise. Am I better off creating the buffer beforehand & always having a set size for it, even if 2/3 of the buffer is unused sometimes? Or is it a better idea to only create the buffer for exactly the size of what it is I need?

I hope I've given ample information... Thanks in advance!

Here is the function in question.

int verifyFiles(DIR *dp, const char *pathroot){
    struct dirent *dir;
    struct stat pathstat;
    //char path[2048];
    int status = 0;

    while((dir = readdir(dp)) != NULL){
        if(!strncmp(dir->d_name, ".", 1))
            continue;

        size_t len = strlen(pathroot) + strlen(dir->d_name) + 2;
        char path[len];
        snprintf(path, sizeof(path), "%s/%s", pathroot, dir->d_name);
        
        // verify shebang is present on the first line of path's contents.
        if(!shebangPresent(path)){
            status = -1;
            break;
        }

        // verify path belongs to the user.
        stat(path, &pathstat);
        if(pathstat.st_uid != getuid()){
            status = -1;
            break;
        }
    }

    return status;
}
Ad

Answer

There is absolutely nothing wrong with having a fixed buffer like that. Don't worry about such small details. The function will allocate 2kB of memory, do it's job and then release it. If that's a problem, then you have bigger problems than this piece of code.

I would only worry about such things in the case of recursive functions. Like if you had something like this:

int foo(int n) 
{
    char buf[2048];
    int r = foo(n-1);
    // Do something with buf and return
}

The above code would quickly eat up the stack for large n. But in your case I really would not worry until you have some proof or at least reasonable suspicion that it's actually causing a problem.

If it was a much larger buffer, say in the order of 100kB, then I would definitely use dynamic allocation. The stack typically has 1MB on Windows and 8MB on Linux. So this is not a matter of "not wasting memory", it's about not blowing up the stack.

Ad
source: stackoverflow.com
Ad