-2

Here is the code that I have:

void InsertStringAt(char *array[], char *s, int pos)
{
    if (pos<array.size())
    {
        *array[pos]=(s *)malloc(sizeof(s));
        strcopy(*array[pos], *s);
    }
    else printf("Position exceeds dimensions of array.");
}

The purpose of this function is to insert the string s into position pos of array[]. Will this code accomplish that?

freelancer05
  • 77
  • 2
  • 6
  • A few notes: C doesn't have "methods" or member function/properties/attributes. When you do `sizeof` of a pointer you get the size of the pointer and not what it points to, use `strlen` to get the length of a string. Once you pass an array to a function it has decayed to a pointer, and you can't get the size of the array. If you have a pointer, and use the dereference operator (e.g. `*s`) then you get the first thing that the pointer points to, in your case a single `char`. And finally, there's no `strcopy` function, perhaps you mean `strcpy`? – Some programmer dude Sep 06 '15 at 17:10
  • 1
    Oh I almost forgot, in C [you should not cast the return of `malloc`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858#605858) (or any function returning a `void *`). – Some programmer dude Sep 06 '15 at 17:11
  • And a lot of what I told you in my first question would have been obvious if you only tried to compile the code (with extra warnings enabled). The error and warning messages should have been quite straightforward. – Some programmer dude Sep 06 '15 at 17:12
  • So just say *array[pos]=malloc(sizeof(s)); – freelancer05 Sep 06 '15 at 17:15

2 Answers2

0

No, this won't do it:

first you cast the result of malloc to (s *) but s is not a type (it is a variable). Then you allocate room the size of s but s is a pointer, so you allocate only room for a pointer.

Then array is declared as an array of pointers to characters. Now you don't need to dereference array anymore when you access a member. The compiler will do that and if you do that, then there is one dereferencing too many.

Do:

array[pos]= malloc(strlen(s)+1);

Lastly, in C there are no classes so array.size is invalid. You must pass the array size as a separate variable.

May I suggest you return an int to indicate everything was successful? E.g. no out-of-memory and pos < size?

Paul Ogilvie
  • 25,048
  • 4
  • 23
  • 41
0

sizeof a pointer simply gives you 8 or 4 (depending on your word size). So to get the length of a string, use strlen() from <string.h>. Also, you needn't cast the return value from malloc. So you'd get:

array[pos] = malloc(strlen(s)+1);

Notice that the dereferencing was removed, since it's already dereferenced.

Also, the function is strcpy() not strcopy(). strcpy() is declared in <string.h>. But strcpy() may lead to buffer overflows, consider making your own:

int safecpy(char *s, const char *t, size_t siz)
{
    size_t i;

    for (i = 1; (*s = *t) && i < siz; ++i, s++, t++)
        ;
    s[i] = 0;       /* null terminate the string */

    return i;
}

C arrays do not know their own size; So pass it as a separate argument. It is recommended that sizes are represented with size_t and not int.

Finally, print error messages to stderr, so they don't get pipelined into another program or redirected into a file:

else fprintf(stderr, "Position exceeds dimensions of array.");
Paul Ogilvie
  • 25,048
  • 4
  • 23
  • 41
lost_in_the_source
  • 10,998
  • 9
  • 46
  • 75
  • `strcpy` will here not lead to buffer overruns as the memory was allocated by `malloc(strlen(s)+1)`. (Don't make it more difficult than necessary.) – Paul Ogilvie Sep 06 '15 at 18:26