I am trying to make a program in Linux in which the command to be executed is passed as a command line argument. I have made a code but it runs file sometimes and sometimes it does not work and i don't know what the problem is.
Here is my code:
#include<stdio.h>
#include<string.h>
#include<unistd.h>
void main(int argc, char* argv[]){
char com[60];
int i;
for(i = 1 ; i < argc ; i++){
strcat(com, argv[i]);
strcat(com, " ");
}
system(com);
}
Your program has undefined behavior.
If the user passes no arguments, the loop is never entered and you do
char com[60];
// ...
system(com);
The contents of com are uninitialized at this point, so the call to system has undefined behavior.
If the user passes arguments, you do
strcat(com, argv[i]);
The contents of com are uninitialized at this point, so the call to strcat has undefined behavior.
To fix this, make com a valid string before the loop:
char com[60];
com[0] = '\0';
That'll sometimes overflow the buffer (very bad), sometimes it'll get the quoting wrong, and with no arguments, the buffer will be uninitialized (=> undefined behavior).
When a shell runs your program, it'll de-quote strings and interpolate $-variables. To do this forwarding robustly, you'd need to requote, in addition to checking against buffer overflow. That would be a bit tedious for in a short program.
The easier thing would be to simply posix_spawn or fork/exec with the argv array directly:
int status;
pid_t pid;
if (0>(pid=fork())) return -1;
if(0==pid){
execvp(argv[1],argv+1);
_exit(127);
}
while(0>(waitpid(pid,&status,0)))
if (EINTR==errno) continue; else abort(); /*shouldn't happen*/
Now, a real system() would additionally ignore SIGINT/SIGQUIT in the parent and block SIGCHLD in the parent before the status is reaped. posix_spawnp would also be preferred.
To copy that right, you can start off with the musl libc's implementation of system, and modify it to create a
int my_system(char const *file, char *argv[i]);
that'll skip the shell (and with it, the need to create a string with correct quoting and buffer overflow checking):
#include <unistd.h>
#include <stdlib.h>
#include <signal.h>
#include <sys/wait.h>
#include <spawn.h>
#include <errno.h>
#include <pthread.h>
//extern char **__environ;
extern char **environ;
int my_system(char const *file, char *argv[])
{
pid_t pid;
sigset_t old, reset;
struct sigaction sa = { .sa_handler = SIG_IGN }, oldint, oldquit;
int status = -1, ret;
posix_spawnattr_t attr;
//pthread_testcancel();
//if (!cmd) return 1;
sigaction(SIGINT, &sa, &oldint);
sigaction(SIGQUIT, &sa, &oldquit);
sigaddset(&sa.sa_mask, SIGCHLD);
sigprocmask(SIG_BLOCK, &sa.sa_mask, &old);
sigemptyset(&reset);
if (oldint.sa_handler != SIG_IGN) sigaddset(&reset, SIGINT);
if (oldquit.sa_handler != SIG_IGN) sigaddset(&reset, SIGQUIT);
posix_spawnattr_init(&attr);
posix_spawnattr_setsigmask(&attr, &old);
posix_spawnattr_setsigdefault(&attr, &reset);
posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETSIGDEF|POSIX_SPAWN_SETSIGMASK);
#if 0
ret = posix_spawn(&pid, "/bin/sh", 0, &attr,
(char *[]){"sh", "-c", (char *)cmd, 0}, environ);
#else
ret = posix_spawnp(&pid, file, 0, &attr, argv, environ);
#endif
posix_spawnattr_destroy(&attr);
if (!ret) while (waitpid(pid, &status, 0)<0 && errno == EINTR);
sigaction(SIGINT, &oldint, NULL);
sigaction(SIGQUIT, &oldquit, NULL);
sigprocmask(SIG_SETMASK, &old, NULL);
if (ret) errno = ret;
return status;
}
Now with my_system, you have no need for quoting and you can simply call it with
my_system(argv[1], argv+1);
com:char com[60] = {0};mainshould returnint.systemat all! Useexecinstead, possibly in conjunction withfork.execthe command once (and probably do not care about the exit value), just useexecvewith&argv[1]as the pointer to the first argument in the list. Simple as that