Need some advice and feedback on my C code calling Bash functions

Question

My intent is to use Bash functions defined in functions.sh in a C program. I am doing this so that I don't have to rewrite the Bash functionality again in C. I want to use one common library for functions I need. Here is my C code and functions.sh.

#include <stdlib.h>
#include <sys/wait.h>
#include <stdio.h>

void shellCall(char * command)
{
    int status, commandexitcode=0;

    if(command == NULL)
    {
        printf("NULL command string sent\n");
        exit(1);
    }

    status = system(command);

    if(status == -1)
    {
        printf("Error during call, value %d\n", status);
        exit(1);
    }

    if (WIFEXITED(status)) 
    {
        commandexitcode =  WEXITSTATUS(status);
        if (commandexitcode != 0)
        {
            printf("non zero exit code: %d\n", commandexitcode);
            exit(1);
        }
    } 
    else if (WIFSIGNALED(status)) 
    {
        printf("killed by signal: %d\n", WTERMSIG(status));
        exit(1);
    } 
    else if (WIFSTOPPED(status)) 
    {
        printf("stopped by signal: %d\n", WSTOPSIG(status));
        exit(1);
    } 
    else if (WIFCONTINUED(status)) 
    {
        printf("continued, unexpected..bailing\n");
        exit(1);
    }
}

int main(int argc,char **argv)
{
    char  command[100]; //command
    int i = 0;
    for (i = 1 ; i <= 10 ; i++)
    {
         sprintf( command, "source $PWD/functions.sh ; i=%d ; myfunc %d", i,i );
         shellCall(command);      
    }
}

Please note that here I have only shown a very simple example of the way I am using functions in functions.sh. In reality functions.sh has many functions, some of which implement decent logic.

---

#!/bin/sh
# functions.sh

myfunc()
{
    echo "I received $1"
}

Question: I need feedback on the following aspects.

1. Is it a bad programming practice (something that will make experienced and senior developers mad when they see this) to use system() calls to call functions in functions.sh as I have done?
2. What are the reasons (technical and other) for which it is considered a bad programming practice?
3. What changes should I make to my C code so that experienced and senior developers will find it acceptable?

Answer 1

Comments

Because your question is marked clean-code and I see that you have commented your declaration of the variable command with the comment command, let me make a note about comments. There are good comments, and bad comments; in my experience, a very high numbers of comments written by many developers are bad. For a thoughtful exposition of comments, see Bob Martin's chapter on them in Clean Code (an invaluable book). In short, if you are going to insert a comment, make sure it actually adds something. Then, make sure you can't add that information into the code itself, by renaming variables, etc. Then, if you can't, carefully craft your comment.

Functions

Because you are asking about the general cleanliness of your code, let me advice that you use smaller functions. There are many benefits to small functions, which I will not get into here; many of the best software books discuss this, including Martin's book, mentioned above.

Magic Numbers

Replace numbers like -1 and 10 in your code with a constant that has an explicit name explaining these numbers. This increases the readability and maintainability of your code.

Stdout and Stderr

In Unix, C and C-based languages, you have three streams attached to your process by default: standard in, standard out, and standard error. Don't write error messages to standard out—write them to standard error.

Calling Bash from C

Consider that calling Bash...

• ...makes your code much, much less portable, by tying it down to a system with Unix utilities—and more specifically than that, one that has Bash—and more than that, one that has a version of Bash which supports all the features you decide to use.
• ...makes your code harder to test
  • by creating external dependencies. If you are not familiar with the problem of dependencies, try reading up on it. Roy Osherove, in The Art of Unit Testing, and Michael Feathers, in Working Effectively with Legacy Code, both have much discussion of it.
  • Bash scripts do not have the debugging tools for than that C has (Valgrind, GDB, IDEs, etc).
• ...makes your code harder to read:
  • by lessening its homogeneity
  • by making the user move around move between various source files
  • by lessening your IDE's ability to aid you in traversing the code, since some of the calls are embedded in strings as calls to an external
  • because Bash is often even harder to read than C
• ...makes your code harder to maintain, because of being harder to test and read.
• ...is unnecessary, because under the covers Bash is just calling the Unix C API anyway.
• ...is artistically distasteful— it smells like a kludge.
• ...adds a failure point to your code. Just look how many possible error messages you output, and then abort!
• ...presumably, this will also be a performance hit because of the overhead of running Bash.

I would strongly advise you not to use sys calls unless you are calling a very complex external application that would be difficult to reproduce in your C code in reasonable time and doesn't have a C API (and serious applications are not generally written in Bash) or you are performing an extremely low-level function that there is no C API for. Work for homogeneity—it's much cleaner.

Answer 2

I think objections to using system are often about the security holes it opens up. The call is generally frowned on. system spawns a copy of the shell (presumably your default shell) and this shell runs the script, using the search path to find it etc. I imagine it is because of this intermediate process and the possibility for interference with it or the command it runs that the security problems arise, but I'm not the right person to give you details. Perhaps ask on Stack Overflow, without the code example, just a general question about the desirability of system for your purposes.

As for your code, it is a lot better now than when first posted. A few points though:

• the parameter to shellCall should be const
• commandexitcode is verbose and could be declared closer to it point of use
• possible buffer overflow printing command in main