0

I want to remove the i-th member of a string array, and bring every member which comes after it one place prior (the i+1-th member to i and so on). I came up with the following code: 

    for (int j = i; j < arrSize - 1; j++) {
        strcpy(members[j],members[j+1]);
    }
    free(members[arrSize-1]);

But that got me thinking that it might be wrong. For example, if the i-th place member contains the name "John", while the (i+1)-th place members contains the name "Joshua", which means each string is in a different length, would there be any memory leaks or any problem? Thanks in advance!

EDIT: The definiton of members: 

members = malloc(maxMembersNum * sizeof(char*));
Improve this question
5
  • and what exactly is a string array? I mean, show us the definition of members. Commented Apr 24, 2015 at 21:41
  • how do you allocate for members? Commented Apr 24, 2015 at 21:42
  • There's no way to answer this question without seeing the declaration of members and how it's built. C has multiple ways to do this. Commented Apr 24, 2015 at 21:42
  • free(members[i]);for (int j = i; j < arrSize - 1; j++) { members[j] = members[j+1]; } ` Commented Apr 24, 2015 at 21:50
  • If arrSize is actually a size then the code has a buffer overflow on the last iteration of the for-loop, and it should be arrSize-2 in the for-loop Commented Apr 24, 2015 at 22:00

3 Answers 3

Reset to default
1

Rather than copy the contents of the strings, why not move the pointers around? That is:

for (int j = i; j < arrSize - 1; j++) {
    char *temp = members[j];
    members[j] = members[j+1];
    members[j+1] = temp;
}
free(members[arrSize-1]);
Improve this answer
Sign up to request clarification or add additional context in comments.

7 Comments

That only works if there are pointers. OP has not shown us that.
@LeeDanielCrocker They must be, else the call to strcpy and the free at the end make no sense.
Actually, there's no possible code in which they both make sense. Either it's an array of pointers, in which case he needs the free but not the strcpy, or it's a 2d char array, in which case he needs strcpy but not free.
To expand on this answer, if the order of the strings in members does not matter, then you can also perform a swap with i and the last member only, rather than swapping all items after i. Also, if arrSize is actually a size then the code has a buffer overflow, and it should be arrSize-2 in the for-loop.
The order does matter. In addition, why is there a buffer over follow? for j =arrSize-2 I'll access arrSize-2 and arrSize-1.
|
1

As the comments said, the definition of members determines the outcome of the function.

If it is:

char* members[];

then it is an array of pointers, using strcpy will overwrite memory and you will eventually crash unless all of those buffers are allocated and the same size. In this case, you can just copy the pointers with 

members[j] = members[j+1]

if it is:

std::string members[];

then you can treat it as a normal array and dispense with strcpy and free (just use =).

if it is:

char members[80][80]; // fixed size pre-allocated buffer

then your code will work, but free will not.

Improve this answer

2 Comments

std::string members[];...in C? how?
Sorry, that would require C++. I missed the OP's restriction to 'C' only.
-1

It would be simpler if lines and arrSize were members of a structure that you could pass in as a unit, but absent that:

void remove_by_index(char **lines, int line, int *arrSize) {
    if (line >= *arrSize) return;
    *arrSize -= 1;

    if (lines[line]) free(lines[line]);
    memmove(lines + line, lines + line + 1, ((*arrSize - line) * sizeof(char *)));
    lines[*arrSize] = NULL;
}

Strictly speaking, either arrSize has to change to reflect deletion, or else the deleted cell has to be marked with NULL, but it's not necessary to do both as I have done here (though it's not a bad idea).

Also note that this will fail if two members of the array point to the same string, as will the other answers here--for that you'll need a more complex data structure.

Improve this answer

1 Comment

free(lines[*arrSize]); is wrong. free should apply to lines[line] before memmove.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.