Combine two strings into one in C programming

Question

I'm having difficulty combining two string in C Programming, I want to be able to take an input files name from the command-line parameters and add .out to the files name as the output files new name. e.g. Test1.txt -> Test1.txt.out

The code below produces a segmentation fault for an unknown reason.

int main(int argc, char** argv)
{
    char fileName_Out[200];
    Consortium *con1;   
    int i;

    for(i=0; i<argc; i++)
    {
        strcpy(fileName_Out, argv[i]);
        strcat(fileName_Out, ".out");
        con1 = readConsortium (argv[i]);
        writeNetWorth (fileName_Out, con1);
    }


    free(con1->core);
    free(con1->associate);
    free(con1);
    con1->core = NULL;
    con1->associate = NULL;
    con1 = NULL;

    return 0;
}

Update with whole code:

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

typedef struct {
    char code[4];
    float sharePrice;
    int shares;
    float assetValue;
    float debts;
} Company;

typedef struct {
    int numCore;
    int numAss;
        Company* core;
        Company* associate;
} Consortium;

Consortium *readConsortium (char* fileName) {

    Consortium *con1 = (Consortium*)malloc(sizeof(Consortium));
    int i;
    FILE *source_f = fopen(fileName, "r");

    if(source_f == NULL)
    {
        con1 = NULL;
    } else {

    fscanf(source_f, "%d %d", &(con1->numCore), &(con1->numAss));  

    con1->core = (Company*)malloc(sizeof(Company)*(con1->numCore));
    con1->associate = (Company*)malloc(sizeof(Company)*(con1->numAss));

    for(i = 0; i < con1->numCore; i++)
    {
        fscanf(source_f, "%s %f %d %f %f", con1->core[i].code, &con1->core[i].sharePrice, &con1->core[i].shares, &con1->core[i].assetValue, &con1->core[i].debts); 
    }

    for(i = 0; i < con1->numAss; i++)
    {
        fscanf(source_f, "%s %f %d %f %f", con1->associate[i].code, &con1->associate[i].sharePrice, &con1->associate[i].shares, &con1->associate[i].assetValue, &con1->associate[i].debts); 
    }

    }

    fclose(source_f);

    return con1;
}

void writeNetWorth (char* fileName_Out, Consortium *con)
{
    int i;
    float netWorth;
    FILE* target_f = fopen(fileName_Out, "w");

    for(i = 0; i < con->numCore; i++)
    {
        netWorth = (con->core[i].sharePrice * con->core[i].shares) + con->core[i].assetValue - con->core[i].debts;
        fprintf(target_f, "%s:%12.2f\n", con->core[i].code, netWorth); 
    }

    for(i = 0; i < con->numAss; i++)
    {
        netWorth = (con->associate[i].sharePrice * con->associate[i].shares) + con->associate[i].assetValue - con->associate[i].debts;
        fprintf(target_f, "%s:%12.2f\n", con->associate[i].code, netWorth); 
    }

    fclose(target_f);
}

/* int main(void)
{
    char fileName[200];
    char fileName_Out[200];
    Consortium *con2;

    scanf("%s %s", fileName, fileName_Out);

    con2 = readConsortium (fileName);
    writeNetWorth (fileName_Out, con2);

    free(con2->core);
    free(con2->associate);
    free(con2);
    con2->core = NULL;
    con2->associate = NULL;
    con2 = NULL;
    return 0;   
}*/

int main(int argc, char** argv)
{
    char fileName_Out[200];
    Consortium *con1;   
    int i;

    for(i=1; i<(argc+1); i++)
    {
        strcpy(fileName_Out, argv[i]);
        strcat(fileName_Out, ".out");
        con1 = readConsortium (argv[i]);
        writeNetWorth (fileName_Out, con1);
    }


    free(con1->core);
    free(con1->associate);
    free(con1);
    con1 = NULL;

    return 0;
}

You access `con1` via `->` after freeing it, which you shouldn't do. just `con1 = NULL;` is enough. — M Oehm, Nov 17 '14 at 07:09
You probably want to start with `argv[1]` because `argv[0]` is the name of your program. — JS1, Nov 17 '14 at 07:12
With GDB I get a SIGSEGV message as soon as the program is run, could this mean the segfault occurs at the beginning of the program? — IamTrent, Nov 17 '14 at 07:25
without a breakpoint, you'll get a segfault always `as soon as the program is run`, no matter at which line the current execution is.. — Sourav Ghosh, Nov 17 '14 at 07:26
@SouravGhosh Thanks GDB shows SIGSEGV occurs at strcpy, it must be this which is causing the problems? — IamTrent, Nov 17 '14 at 07:30
@IamTrent did you try `memset()`-ing as told in my other comment? — Sourav Ghosh, Nov 17 '14 at 07:31

Sourav Ghosh · Accepted Answer · 2014-11-17T07:50:26.987

1

Two things.

Are you sure, you're not running out of memory while using fileName_Out[200] ? From the man page of strcat()

the dest string must have enough space for the result. If dest is not large enough, program behavior is unpredictable;
~~You have never seem to allocate memory to con1 pointer in the code you've shown. [Considering you didn't show us readConsortium() definition].~~
~~I think you should refrain from free-ing con1-> .. as in your main() there is no allocation for con.~~

Edit:

Your problem was somewhere else in the code. with the for loop specifying

for(i=1; i<(argc+1); i++)

you're running out of bounds. Change that condition to

for(i=1; i<argc; i++).

Remenber, the nth element in array will always have an index of n-1.

edited Nov 17 '14 at 07:50

answered Nov 17 '14 at 07:08

Sourav Ghosh

133,132
16
183
261

con1 is allocated memory within writeNetWorth function, removing the strcat and strcpy function doesn't show a segfault, the input fileNames are less than string length of 99. – IamTrent Nov 17 '14 at 07:10
@IamTrent You have to include those functions in your question or nobody will be able to answer it. Most likely the bug are within those functions. – Lundin Nov 17 '14 at 07:14
The rest of the functions work correctly when the strcpy and strcat are removed, so it's doubtful to be the problem. I will include them anyway. – IamTrent Nov 17 '14 at 07:16
What would be the appropriate place to free the struct? – IamTrent Nov 17 '14 at 07:20
@IamTrent maybe you can just try to initialize the `char fileName_Out[200];` by using `char fileName_Out[200] = {0, };` or `memset()`? – Sourav Ghosh Nov 17 '14 at 07:28
Tried it and the problem wasn't fixed, still the same segfault error. – IamTrent Nov 17 '14 at 07:33
1

@IamTrent did you notice `for(i=1; i<(argc+1); i++)`? shouldn't it be `for(i=1; i – Sourav Ghosh Nov 17 '14 at 07:42
@IamTrent also you may want to re-think your approach. to append a `.out, a for loop is _overkill_ and may very well lead to incorrect filenames. – Sourav Ghosh Nov 17 '14 at 07:44
It's not by my choice, a worksheet asked me to do it in this way. – IamTrent Nov 17 '14 at 07:45
Also could you tell me where would be the better place to free the con struct, seeing as the struct is used in both the other functions? – IamTrent Nov 17 '14 at 07:46
1

@IamTrent with the complete code you've posted, it looks fine. I'll edit my answer. :-) – Sourav Ghosh Nov 17 '14 at 07:47

score 0 · Answer 2 · answered Nov 17 '14 at 08:04

To me it seems as if con1 becomes NULL but you try to use that pointer in writeNetWorth without checking for NULL.

for(i = 0; i < con->numCore; i++)

Will cause a segfault when con is NULL.

The reason that con1 becomes NULL is most likely that you are unable to open yourprogram.exe.out which you got from from "argv[0].out".

As a side note it also seems as if your program has memory leaks, dynamically allocating memory in a loop without freeing memory in the same loop.

Combine two strings into one in C programming

2 Answers2