Why does this malloc give me a segmentation fault?

Code:

---


    f = fopen("test1.txt", "r");
    if (f == NULL) {
        printf("Could not open file.\n");
        exit(1);
    }
    //Memory allocation for output file name
    output_titles = (char*)malloc(sizeof(char*)*(strlen(strcat("test1.txt", ".titles"))+1));

    //Creation of output file name
    strcpy(output_titles, strcat("test1.txt", ".titles"));

---

I'm getting a segmentation fault, and it seems to be happening because of wrong use of malloc. But why is this wrong?

Possible reason in link
FAQ > Casting malloc - Cprogramming.com

@Stahta,

I tried what is suggested but I'm still getting the same. :/

Malloc isn't giving you a segfault. This is:

Code:

---

strcat("test1.txt", ".titles")

---

Why aren't you just doing:

Code:

---

strlen( "test1.txt.titles" )

---

Quzah.

Because normally, the user will name the file (which I'm stating as "test1.txt") as an argument. Meaning that, to run this, the use will type: /nameofprog test1.txt

So in other words, the program can't initially be told how long the name of the input file will be. I guess I couldn't just write strlen("argv[1].titles") or something like that.

I suggest then that you get the part where they take input working first. Because your test case is broken by you trying to strcat string literals.


Quzah.

Code:

---

f = fopen(argv[1], "r");
    if (f == NULL) {
        printf("Could not open file.\n");
        exit(1);
    }

---

Is that what you're referring to? Because I think this part is working.

The point is that you can't concatenate anything onto a string literal, since it does not have the extra space allocated to hold anything else. So first strcpy "test1.txt" to a char array with enough space to hold your final string.

Also, when you malloc your memory, you actually want sizeof(char) not sizeof(char*).

You have test1.txt\0 and .titles\0. Strcat supposedly creates test1.txt.titles\0, or "test1.txt.titles". So why am I unable to use strcat(argv[1], ".titles") + 1 as the space needed, and then do strcpy(output_titles, strcat("test1.txt", ".titles"))?

Please follow and read link
strcat - C++ Reference

The destination must be a NON constant and have enough space to hold both C-strings followed by a zero byte.

NOTE: You should NEVER write to the argv memory; doing so is undefined!

Tim S.

Code:

---

    output_titles = (char*)malloc(sizeof(char) * (strlen(argv[1]) + strlen(".titles") + 1));
    if (output_titles == NULL) {
        printf("Memory allocation error.\n");
        exit(1);
    }
   
    output_prices = (char*)malloc(sizeof(char) * (strlen(argv[1]) + strlen(".prices") + 1));
    if (output_prices == NULL) {
        printf("Memory allocation error.\n");
        exit(1);
    }
   
   

    strcpy(output_titles, argv[1]);
    strcat(output_titles, ".titles");
    strcpy(output_prices, argv[1]);
    strcat(output_prices, ".prices");

---

I tried doing this but to no avail. Still getting a segmentation fault.

Works for me.

Code:

---

test.titles
test.prices

Process returned 0 (0x0)  execution time : 0.008 s
Press any key to continue.

---

Code:

---

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

int main (int argc, char *argv[]) {

    char *output_titles;
    char *output_prices;

    if (argc <= 1){
        printf("Syntax error.\n");
        exit(1);
    }

    output_titles = (char*)malloc(sizeof(char) * (strlen(argv[1]) + strlen(".titles") + 1));
    if (output_titles == NULL) {
        printf("Memory allocation error.\n");
        exit(1);
    }

    output_prices = (char*)malloc(sizeof(char) * (strlen(argv[1]) + strlen(".prices") + 1));
    if (output_prices == NULL) {
        printf("Memory allocation error.\n");
        exit(1);
    }



    strcpy(output_titles, argv[1]);
    strcat(output_titles, ".titles");
    strcpy(output_prices, argv[1]);
    strcat(output_prices, ".prices");

    printf("%s\n", output_titles);
    printf("%s\n", output_prices);

    return 0;
}

---

Tim S.

You're definitely right, it's working, thank you! :D
I replaced argv[1] with "test1.txt" to be able to use the gdb and turns out the segmentation fault occurs later on, at a different stage.

It doesn't do you any good to be handed code that works, if you still don't comprehend why it works.


Quzah.

I do get it, now!