Need some advice and feedback on my code's design

Question

I am looking for feedback on the design of my program.

I have a shell script call function.sh that defines a lot of helper functions. My intent is to use those 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 (in functions.sh) again in C. I want to use one common library for functions I need (which is functions.sh).

Here is my C code and functions.sh
C code:

#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))
    ...
}


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);      
    }
}

functions.sh:

#!/bin/sh
# functions.sh

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

Please note that 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.

---

My Questions - 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. If it is a bad practice, 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

Calling 'system()' has a huge amount of overhead. The OS has to spin up a new shell for each such call. Kudos for having done error checking on the return value of system(), but you can see how much more complex it makes your program. Furthermore, you need to add still more error checking. You need to check that the shell scripts are where you think they are before you call them with system(). If they're not where you expect them to be, or have the wrong permissions, your program is going to fail with user unfriendly messages.

You are making distribution and configuration of your program more complex. Not only do you have to distribute a binary, but you have to distribute the shell scripts as well and make sure they end up in the correct directory.

You don't explain why you are doing it this way, but in the absence of other information I'd suggest turning the whole thing inside out. Write a shell script that calls a few C programs to do any computationally intensive work.

Answer 2

I am writing a C program because bash shell does not allow dynamic memory allocation and I need that for my logic ( although I have not shown that above).

At a high level, this appears to be a conglomeration of a C and shell scripts where one is controlling the other - the exact nature of where the logic exists isn't well defined in the example.

The one classic example of such a design is a shell script (with the application logic in it) which is invoking a number of compiled C programs that do what the shell script cannot. To an extent this is 99% of all shell scripts - invoke something, pipe it to grep, pipe it to cut, set a variable from that, invoke something based on that variable, pipe that output to awk and sed, etc...

Such a design is quite reasonable and common and nothing for one to be extremely offended about.

Another approach is where the application is a single complied program that is doing whatever it is doing (listening for network connections, chugging away at calculation intensive bits, formatting xml and tossing it on a message queue, displaying a gui, etc...). Sometimes this application finds itself in need of interacting with the underlying system - for example, in attempting to put a message on a queue it may have found that that demon has died and needs to restart it - set a bunch of environment variables, sudo /etc/init.d/amqd start. While it is possible for this compiled program to instead fork a copy of itself that then modifies its environment and does system calls, that can be expensive for the compiled application and is likely prone to more bugs than something that is easier to understand and maintain. It is perfectly for this model too and while some people may grumble about multiple parts of the application, it isn't a serious offense.

Let us look at the quote again and put this into context of the code sample in the question.

I am writing a C program because bash shell does not allow dynamic memory allocation and I need that for my logic ( although I have not shown that above).

It appears that the application as a whole has its logic split between the functions in functions.sh and the compiled C code. There is a state machine (of sorts) that exists in the combination of the two pieces. The application logic doesn't appear to exist solely in one part or the other. The compiled program invokes a shell script with a function that then runs and its output is used to determine which shell to run next. The compiled program exists as a dynamic data store for the shell programs.

This is poor design.

Changes in one affect changes in the other. There is no single point of responsibility for the logic. Maintaining two sets of languages with interdependence between the two can be challenging to keep in working order, test, and deploy.

---

I am a fan of the camel. This will color my suggestion.

Any shell script longer than 10 lines should be rewritten in perl.

The original justification for the split design was that bash didn't have acceptable performance with dynamic arrays memory. Ok, so don't use bash. However, it appears there is some desire to interact with the system at a higher level than C provides. Ok, so don't use C. Use perl (or python, or if you must, ruby).

These languages excel doing the complex processing that shell scripts alone have difficulty doing without having to go to a C program. In many cases, one can do everything that is traditionally done in shell (cut, grep, awk, sed, tr, open file, write to file, etc... ) without having to fork another process. Furthermore, having a single application that contains all of the logic within itself is a superior design than trying to split it between two dissimilar ones.

Yes, it does mean rewriting functions.sh in perl (though at the moment, the entirety of the provided code is the builtin print). It also means that after this is done, you will have a library of calls in a nice, self contained module.