-1

I am working on a project in C language and my code keeps crashing of fputs command. This is the code:

void entry_file_creation(int lc, int error_flag_line, char *source_file_name)
{
    int j = 0;
    FILE *entry_ptr = malloc(sizeof(FILE*));
    if (entry_ptr == NULL)
    {
        fprintf(stderr, "\nError, please try again");
        exit(-1);
    }

    while (j < lc)
    {
        if (error_flag_line == 1)
        {
            ++j;
            continue;
        }
        if (label_table[j].extern_entry == 2)
        {
            source_file_name = strtok(source_file_name, ".");
            entry_ptr = fopen(strcat(source_file_name, ".ent"), "w");
            if (entry_ptr == NULL)
            {
                fprintf(stderr, "\nError: something went wrong with creating entry file - '%s'. please try again\n\n", source_file_name);
                exit(-1);
            }
            printf("\nLabel: %s \n", label_table[j].name);
            fputs(label_table[j].name, entry_ptr);
            fputs("\t", entry_ptr);
            fprintf(entry_ptr, "%d\n", label_table[j].address);
        }
        ++j;
    }
    free(entry_ptr);
}

Code crashes on line:

fputs(label_table[j].name, entry_ptr);

The error is corrupted size vs. prev_size: 0x08bfc5b0 please help.

WedaPashi
  • 3,561
  • 26
  • 42
Eitan Ratner
  • 29
  • 3
  • 2
    You don't need to separately allocate space for `FILE * entry_ptr` - this memory is lost when `fopen()` returns the FILE pointer. And when you're done, you `fclose(entry_ptr)` rather than the `free()`. Also, this appears to re-open the file on every loop, so it's not clear what this is trying to accomplish. – Steve Friedl Aug 30 '23 at 15:18
  • 1
    What are you trying to do, exactly? – Neil Aug 30 '23 at 15:33
  • 1
    `FILE* x = fopen(...)` is the pattern to use. `malloc()` and `free()` play absolutely no part in those sorts of operations. – tadman Aug 30 '23 at 15:36
  • Also: the `strcat(source_file_name, ".ent")` is dangerous: we don't know how much room is available on that string, and depending on circumstances could actually corrupt the stack. If the old extension is `.x` and the new is `.ent`, then it's writing two bytes past the string. Danger. – Steve Friedl Aug 30 '23 at 15:39
  • *"fputs causing error corrupted..."* means you invoke *Undefined Behavior* elsewhere in your code... – David C. Rankin Aug 30 '23 at 18:35
  • Please, edit your question to give a [Minimal, complete and reproducible example](https://stackoverflow.com/help/minimal-reproducible-example) Providing snipets, undefined `struct`s and the like you can only confuse people. Most probable thing is that your structure members have not been correctly initialized as null terminated strings. – Luis Colorado Sep 02 '23 at 19:02

1 Answers1

3

This allocation doesn't make sense for only a single file handle:

FILE *entry_ptr = malloc(sizeof(FILE*));

Note that it is semantically-incorrect even if you wanted to dynamically allocate a single handle. The proper way to (needlessly) allocate a single handle would be:

FILE **entry_ptr = malloc(sizeof(FILE *)); // *entry_ptr = fopen()

Then you do:

entry_ptr = fopen(strcat(source_file_name, ".ent"), "w");

It effectively loses the pointer to the allocated memory, overwriting it with the returned handle from fopen.

Further, free(entry_ptr) is requested to free a block that it has never allocated.

You probably want FILE *entry_ptr = fopen(...) in every iteration of the loop, closing it with fclose at the end of the iteration.

Blagovest Buyukliev
  • 42,498
  • 14
  • 94
  • 130
  • 1
    +1, also -- `FILE *entry_ptr = malloc(sizeof(FILE*));` the allocation wouldn't make sense for any use case either. It will allocate 4-bytes which is typically the size of a pointer, thats 4-bytes leaked permanently. – WedaPashi Aug 30 '23 at 15:25
  • 1
    @WedaPashi Maybe 10 years ago. On any 64-bit system it's 8 bytes. – tadman Aug 30 '23 at 15:36
  • 1
    @tadman -- Well, yeah. Fortunately or unfortunately, I work with 8/16/32 microcontrollers in deep embedded stuff, soy mind still works that way – WedaPashi Aug 30 '23 at 17:37
  • I may be missing something, but if we are opening and closing after a set of iterations why allocate to begin with? Also if using the pointer-to-pointer, after allocation, wouldn't that require `*entry_ptr = fopen(strcat(...))`? – David C. Rankin Aug 30 '23 at 18:33