CODEWITHSUNDEEP
carrayssegmentation-faultstrcpy
sampson7185
sampson7185

Reputation: 23

Getting a segmentation fault when trying to use strcpy() to a 2D char array?

I'm trying to write a function that takes in two strings, concatenates them, and then writes them to a 2D array. I have been messing around with different things and looking at other posts to no avail. The seg fault seems to occur in the last strcpy when I'm trying to write the the concatenated string to the char array. If that line is commented out the program seems to work just fine.

void saveToArray(char *command1, char *command2)
{
    char *saveCommands[30][100];
    int *commands = malloc(sizeof(int));
    char concatCommand[30];

    strcpy(concatCommand, command1);
    strcat(concatCommand, " ");
    strcat(concatCommand, command2);
    strcpy(*&saveCommands[0][*commands], concatCommand);
    *commands += 1;
}

I apologize in advance for any formatting issues as this is my first post and thanks for any answers!

Upvotes: 1

Views: 1032

Answers (2)

Travis Rodman
Travis Rodman

Reputation: 637

I took the liberty to re-write your code a bit... too many characters make my eyes hurt... at any rate, this code works. It was mainly the strcpy dereference that was causing the issue.

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

void cmds(char *c1, char *c2) {
    char  sc[30][100];
    int   c = 0;
    char  cc[30];

    strcpy(cc,  c1);
    strcat(cc, " ");
    strcat(cc,  c2);
    strcpy(&sc[c][0], cc); // problem was here...
    printf("%s\n", &sc[c][0]);
}

int main() {
    char *c1 = "c001";
    char *c2 = "c002";

    cmds(c1, c2);
    return 0;
}

Upvotes: 0

Peter Enns
Peter Enns

Reputation: 610

There are a number of scary things going on in this function. For starters...

int *commands = malloc(sizeof(int));

You don't free this or return it, so it is 100% a memory leak. I see no reason why you should want to allocate this integer on the heap rather than the stack.

char concatCommand[30];

strcpy(concatCommand, command1);
strcat(concatCommand, " ");
strcat(concatCommand, command2);

This could potentially overflow your buffer.

strcpy(*&saveCommands[0][*commands], concatCommand);

*& is a no-op (it doesn't do anything). You don't copy anything into commands, so *commands could be anything. Finally, the pointers in saveCommands aren't initialized so strcpy(saveCommands[x][y], "some string") will basically always segfault in practice. I think you might want this instead:

char saveCommands[30][100]
...
strcpy(&saveCommands[0][*commands], concatCommand);

Upvotes: 1

Related Questions