-1

I am trying to pass command line arguments which I then concatenate appropriately to generate shell commands so that I can run them using system() (I know it is not advisable and there are better ways but I have been asked to do it in this way only). But there's some problem in concatenation of the strings that I pass Here is the code (I have printed everything at every step to get a clear understanding and no I haven't yet written the system() calls,first I need to sort this concatenation out):

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int main(int argc, char *argv[]) {
     char* path=argv[1];
     char* oldName=argv[2];
     char* newName=argv[3];
     char* command1="cd "; 
     char* command2="ren ";
      printf("\n\n%s\n%s\n%s\n%s\n%s\n",command1,command2,path,oldName,newName);
    strcat(command1,path);
    printf("\n\n%s\n%s\n%s\n%s\n%s\n",command1,command2,path,oldName,newName);
    strcat(oldName," ");
    strcat(oldname,newName);
    printf("\n\n%s\n%s\n%s\n%s\n%s\n",command1,command2,path,oldName,newName);
    strcat(command2,oldName);
    printf("\n\n%s\n%s\n%s\n%s\n%s\n",command1,command2,path,oldName,newName);

    return 0;
}

However after concatenating command1 to path everything gets messed up.

https://i.stack.imgur.com/vPxxD.png

Roshan Mathews
  • 5,788
  • 2
  • 26
  • 36
Shivam Upadhyay
  • 89
  • 1
  • 1
  • 6

3 Answers3

1

strcat works by copying bytes from the source string to the end of the destination string. The problem is that your destination strings are:

  1. Constant strings (and may not be in writeable memory)
  2. Not long enough to hold the entire result

You should probably create a char buffer like char buffer[1024] to hold the commands and use snprintf to format the commands into the buffer.

EricS
  • 9,650
  • 2
  • 38
  • 34
  • Better, but still not good enough for actual use. Either compute the actual needed size with `strlen`, or make it ridiculously too big, or do real string processing instead of `strcat`. – Potatoswatter Jul 19 '15 at 05:33
  • Yes, in real apps you have to handle arbitrarily sized input strings. Using `strcat` is dangerous if you don't size the buffers properly and this is a common source of security exploits. Using `snprintf` will prevent the buffer overflow, but the output strings will be truncated which is not what you want either. – EricS Jul 19 '15 at 16:59
1

strcat expects the destination to be big enough to hold the result. To quote:

Pointer to the destination array, which should contain a C string, and be large enough to contain the concatenated resulting string.

Roshan Mathews
  • 5,788
  • 2
  • 26
  • 36
-1

You need to allocate space using a character buffer to prevent the strcat from crashing, like this:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int main(int argc, char *argv[]) {
    char* path=argv[1];
    char oldName[256];
    char* newName=argv[3];
    char command1[256];
    char command2[256];
    strcpy(command1, "cd ");
    strcpy(command2, "ren ");
    strcpy(oldName, argv[2]);
    printf("\n\n%s\n%s\n%s\n%s\n%s\n",command1,command2,path,oldName,newName);
    strcat(command1,path);
    printf("\n\n%s\n%s\n%s\n%s\n%s\n",command1,command2,path,oldName,newName);
    strcat(oldName," ");
    strcat(oldName,newName);
    printf("\n\n%s\n%s\n%s\n%s\n%s\n",command1,command2,path,oldName,newName);
    strcat(command2,oldName);
    printf("\n\n%s\n%s\n%s\n%s\n%s\n",command1,command2,path,oldName,newName);

    return 0;
}
Rudi Cilibrasi
  • 885
  • 4
  • 12
  • 2
    256 bytes should be enough for a command line containing a path and a few filenames… right. – Potatoswatter Jul 19 '15 at 05:32
  • Helped a lot.Thank you – Shivam Upadhyay Jul 19 '15 at 05:34
  • @Potatoswatter I know you're being sarcastic, but I want to point out that *single paths* sometimes exceed 256 characters. It's an actual problem on both Windows and *nix. – user253751 Jul 19 '15 at 05:59
  • Yes the 256 char thing is just to get you past the segfaults. It will be valuable to try to understand what's going on here, increase the sizes of the buffers, and eventually figure out how to do it without fixed size buffer limitations. I am glad it helped you. – Rudi Cilibrasi Jul 19 '15 at 06:32