why malloc yields error after long argv arguments?

Question

I have written a program where it reads some filenames and a word from command line arguments. The first argument will be a word and remaining will be filenames. It fills a structure that I have defined. However for small arguments, the program works correctly but for large ones malloc gives corrupted top size error.

Below is just initial code of the program:

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

#define FILENAME_LIM 256
#define WORD_LIM 256

struct data {
    char filename[FILENAME_LIM];
    char word[WORD_LIM];
};

int main(int argc, char *argv[]) {
    if (argc < 3) {
        printf("usage: ./a.out word filename1 filename2 filename3 filename4 filename5 ...\n");
        exit(EXIT_FAILURE);
    }   
    char word[WORD_LIM];
    strcpy(word, argv[1]);
    int files = argc - 2;
    struct data *dataarray = (struct data *)malloc(sizeof(sizeof(struct data) * files));
    if (!(dataarray)) {
        perror("malloc");
        exit(EXIT_FAILURE);                                                                                                                                                                                        
    }   
    for (int i = 0; i < files; i++) {
        strcpy(dataarray[i].filename, argv[i + 2]);
        for (int ii = 0; ii < strlen(dataarray[i].filename) + 1; ii++) {
            printf("filename[%d] = %c (%d), argv[%d] = %d\n",
                   i, dataarray[i].filename[ii], dataarray[i].filename[ii],
                   i, argv[i + 2][ii]);
        }
    }   
    return 0;
}

I tried everything, but when I give some large filename in argv such as "../../../C Programs/chess.c", malloc yields an error. I want to know what is making corrupted top size.

Whats the double sizeof in your malloc call doing? Remove the outer one. — tkausl, Oct 24 '20 at 14:02
And that is not an error, but `ii < strlen(dataarray[i].filename) + 1` is the same as `ii <= strlen(dataarray[i].filename)` — , Oct 24 '20 at 14:11
What correlation is there between count of files, i.e. the value of your `files` variable, and the size of `struct data`? shouldn't the actual size of the file in bytes be part of this memory allocation? — ryyker, Oct 24 '20 at 14:17
[don't cast return of malloc](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). Chage this: `struct data *dataarray = (struct data *)malloc(sizeof(sizeof(struct data) * files));` to this: `struct data *dataarray = malloc(sizeof(*dataarray) * files);` — ryyker, Oct 24 '20 at 14:19

score 1 · Accepted Answer · answered Oct 24 '20 at 14:13

1

sizeof(struct data) * files

is a valid size. Any multiple of a size is a size.

sizeof(sizeof(struct data) * files)

is like saying sizeof(23422). I don't know what that means, and odds are the runtime isn't behaving properly either.

Restrict your use of sizeof(...) to processing type arguments.

answered Oct 24 '20 at 14:13

Edwin Buck

69,361
7
100
138

`sizeof(expr)` is exactly the same as applying `sizeof` to the type of the expression. In the case of `sizeof(23422)`, that would be the same as `typeof(int)` (probably 4), but the value of `sizeof` has type `size_t`, which is usually wider than an `int`. So `sizeof(sizeof(struct data) * files)` is most likely 8. – rici Oct 24 '20 at 15:02
It's usually better to use `sizeof(expr)` (in this case `malloc(sizeof(*dataarray) * files)`) without a cast. That protects you against using the wrong type for the object you are allocating. – rici Oct 24 '20 at 15:08
@rici When one needs to explain what is happening instead of what you want, most of the "readability" battle is lost. – Edwin Buck Oct 24 '20 at 15:09
@edwin: i disagree: everything requires an explanation once. Is `a ^ b` not usable because someone not familiar with C might not know that `^` means xor and not power? For that matter, the difference between `sizeof` and `strlen` is apparently in constant need of explanation, as is the fact that `sizeof` is completely expanded at compile time unless its argument is a VLA. – rici Oct 24 '20 at 15:13
@rici I agree that explanations are necessary. But I have a computer that explains my code through execution, for which I can write unit tests, which never deviate from the computer's explanation while my (or other's) mental models can. I'm glad you explained it, but as I mentioned above, I'm stating that such code should never go into production because it requires so much explanation when a more intentional writing approach (within the C code) is possible. Cheers. – Edwin Buck Oct 24 '20 at 15:23
@edwin: you're entitled to your opinion, of course, but there are many people more expert than me who recommend the code I suggested, perhaps enough to call it the prevailing wisdom. See rykker's comment to the main post, for example. Or [this frequently cited answer](https://stackoverflow.com/a/605856/1566221) . You'll also find this idiom recommended by various style guides (random example: https://engineering.purdue.edu/ece264/16au/code_quality#sizeof_argument). – rici Oct 24 '20 at 16:00
@rici Please listen to the people that are more expert than you. I've been programming C since 1985, and I've never seen a document that indicates `sizeof(...)` is supposed to take a value. It takes a type. The reason it doesn't crash is due to implicit promotion of a value to a type, which is not what is wanted in most cases, and certainly not what is wanted here. If you can find a compelling use case of `sizeof(...)` taking a value that is then promoted to a type, then put it as an answer to a different question, because again it doesn't apply here. – Edwin Buck Oct 24 '20 at 16:39

why malloc yields error after long argv arguments?

1 Answers1