1

Example code, treats argv[1] as a file path, replacing its extension with .png or appending it if none is found:

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

int main(int argc, char **argv) {
    if (argc != 2)
        return 1;

    char *lastdot = strrchr(argv[1], '.');
    size_t len = lastdot ? (lastdot - argv[1]) : strlen(argv[1]);

    char outpath[len + sizeof(".png")];
    // ^ this is the important part

    memcpy(outpath, argv[1], len);
    memcpy(outpath + len, ".png", sizeof(".png"));

    printf("%s\n", outpath);
}

My main concern here is that argv[1] may be just small enough to not cause an error from the OS itself, but then the extra 4 bytes added by the ".png" concatenation will cause a stack overflow when allocating the VLA?

The problem is that I'm not really sure how the sizes of either a member of argv or a VLA are limited.

Apparently constants such as PATH_MAX aren't to be trusted for static array sizes, so it's either this or malloc.

  • VLAs are perfectly legal in C, and you are doing nothing wrong. – Dúthomhas May 29 '22 at 13:20
  • 1
    @landfillbaby Nevermind, I misread (or didn't read, rather) the question. Why do you think this is problematic? – HolyBlackCat May 29 '22 at 13:21
  • I dont think it is a problem in practice because the size of argv is often limited to page size ( usually 4096 bytes). – tstanisl May 29 '22 at 13:21
  • 2
    Any reason you're using `memcpy` instead of `strcpy` and `strcat`? – dbush May 29 '22 at 13:22
  • 3
    `int a[4096];` can overflow stack as well. VLA is ok as long as its size is somehow sanitized. Either by the program or by the implemention/runtime environment. – tstanisl May 29 '22 at 13:24
  • @dbush I already know the lengths at that point and assume it's faster than making libc figure it out again, and I'd need `strncpy` anyway. – landfill baby May 29 '22 at 13:28
  • @landfillbaby, the advantage of `strcpy` and `strcat` (or two `strcpy`s) is code clarity. The `memcpy` version is manifestly less clear than an alternative using string functions would be, based simply on the fact that the question in fact arose. I presume that you think there is a performance gain to be realized, but code clarity is almost surely more valuable in a case such as this one. And no, you would not need `strncpy()`, because you have already taken the care necessary to ensure that there is enough space in the destination array. – John Bollinger May 29 '22 at 13:44
  • `strcpy` followed by `strcat` won't work if there's a `'.'` in `argv[1]` though. this code replaces a file extension if one is found, or appends if it isn't – landfill baby May 29 '22 at 14:00

1 Answers1

1

Your concern is genuine: nothing in the C Standard guarantees that the stack space available to your program will be sufficient to allocate a large VLA for a very long command line argument plus 4 bytes. In practice, on modern operating systems, the stack space is typically at least 1 megabyte and command line arguments are limited to about 100 KB... so you should be safe.

Also beware that some compilers do not support VLAs at all, so allocating from the heap is a more portable approach.

Incidentally, you can modify your program to avoid this issue completely:

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

int main(int argc, char *argv[]) {
    if (argc != 2)
        return 1;

    char *lastdot = strrchr(argv[1], '.');
    int len = lastdot ? lastdot - argv[1] : strlen(argv[1]);

    printf("%.*s.png\n", len, argv[1]);
    return 0;
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • 1
    I guess the example wasn't great, in the real code I would have been `fopen`ing it. I'll accept this as the answer. Thanks – landfill baby May 29 '22 at 14:06