0

I've been trying to copy an array of strings called char* args[] which contains basic shell commands such as ls and sort into another array of strings char* argsNew[] and it is not working properly. below is my code for copying one string of arrays to another using strcpy.

 char *args[2]; //Let's say i had this as my original array
 args[0] = "ls";
 args[1] = "sort";
 char *argsNew[2]; //ad don't place anything in argsNew
 //shouldn't this work?
 for(int k = 0; args[k] != '\0'; ++k)
    { 
      strcpy(argsNew[k], args[k]);
      printf("argsNew[%d] is %s\n", k, argsNew[l]);
    }

When I execute this the printf never executes causing me to assume that there was an error when executing the strcpy command. Am I using strcpy incorrectly? Is there a better and simpler way of performing such a task? Any help would truly be appreciated. Thank you.

Edit: I added some more details to the top. Thank you for your speedy responses.

Kevag6
  • 135
  • 1
  • 1
  • 10
  • Check if `argsNew[k]` points to some meory ot not? Then `args[k]!=NULL` will be the correct condition – user2736738 Dec 10 '17 at 04:24
  • Show full code..easier to help – user2736738 Dec 10 '17 at 04:29
  • Without information about what `args` and `argsNew` are (i.e. their types), AND how they have been initialised, it's not possible to help. However, the comparison `args[k] != '\0'` suggests `args[k]` is a `char`, while `strcpy(argsNew[k], args[k])` suggests it is a pointer to the first element of an array (i.e. something else). Even if a compiler accepts the code, the different usages are inconsistent, and suggest a problem. You need to provide an [mcve] to get more useful advice. – Peter Dec 10 '17 at 04:35
  • show me the full code – Ramana V V K Dec 10 '17 at 04:37
  • @Peter So essentially to get the above code to work all i would have to do is change the '\0' to NULL correct? Since args is an array of strings. Thank you – Kevag6 Dec 10 '17 at 04:42
  • @Kevag6.: No..you are thinking it wrong. Check my answer. Simply changing it to `NULL` wont work. It will invoke undefined behavior – user2736738 Dec 10 '17 at 05:01
  • arg[k] != NULL may lead another problem if the arg[2] is not NULL because you dont know that. In this case since you know the size of args or argsnew, probably i would use something like for(int k = 0; k<2; ++k). But still u may face run time problems with argsnew[] i believe – Sudhee Dec 10 '17 at 05:05
  • @Kevag6 - I didn't suggest that. I simply pointed out (1) a characteristic of your code that suggests you are using variables in incorrect ways, and that (2) you need to provide more information in the form of an [mcve]. When you provide incomplete information in your question - as you have - the most you can hope for is pointers to potential concerns, not definitive answers. – Peter Dec 10 '17 at 05:15
  • "*I've been trying to copy an array of strings called `char* args[2]`*" `args` is *not* an array of strings, but an array of *pointer* (note the `*`). It is an array of *pointer* to `char`. – alk Dec 10 '17 at 10:09

2 Answers2

2

One way to do it would be

char *args[3]; 
args[0] = "ls";
args[1] = "sort";
args[2] = NULL; 
char *argsNew[2]; 
for(int k = 0; args[k] ; ++k)
{ 
    argsNew[k] = strdup(args[k]);
    printf("argsNew[%d] is %s\n", k, argsNew[k]);
}

First of all the looping was wrong, you needed to make it either the last pointer variable point to NULL.(Similar to the char* [] passed to main()). Also you didn't point to any memory earlier that you could copy. Earlier operation you were doing was invoking undefined behavior.

Again the drawback with the earlier solution is that we are always taking an extra space. We can avoid that using this solution.

Here we have used strdup to get the allocated memory and copy string to it. And used sizeof to get the length of the array.

Also the looping can be done like this

for(int k = 0; k < sizeof args/sizeof args[0]; ++k)

Code would be like

char *args[2]; 
args[0] = "ls";
args[1] = "sort";

size_t sz = sizeof args/sizeof args[0];
char *argsNew[sz];
for(size_t k = 0; k < sz ; ++k)
{ 
    argsNew[k] = strdup(args[k]);
    printf("argsNew[%d] is %s\n", k, argsNew[k]);
}
user2736738
  • 30,591
  • 5
  • 42
  • 56
  • Thank you for your help, i tried both solutions given and it works perfectly. I have one final question. Let's say instead of argsNew being an array of char pointers, like above, what if it was just a basic char array for example char argsNew[1024]; Would strcat(argsNew, args[k]); work using the same for loop? I would have to terminate with a '\0' as well correct? Thanks again. – Kevag6 Dec 10 '17 at 16:00
  • @Kevag6.: This is a different thing ..now if you concatenate `s1` with `s2` then `s1` must be null terminated. and so is the case for `s2`. Otherwise it wont work (using `strcat`) Then solution would be a bit different. Try to read the manual of `strcat` and basics of c-string. You will get a better idea. Neither comment-box nor the answer to this question, would be a good place to elaborate this. – user2736738 Dec 10 '17 at 16:18
  • much appreciated man – Kevag6 Dec 10 '17 at 16:36
-1

Try this, can solve the issue

 for(int k = 0; args[k] != null; ++k)
    { 
      strcpy(argsNew[k], args[k]);
      printf("argsNew[%d] is %s\n", k, argsNew[k]);
    }
Ramana V V K
  • 1,245
  • 15
  • 24