Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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?

share|improve this question
6  
Might be a better fit for codereview.stackexchange.com –  P.Brian.Mackey Feb 1 '13 at 20:33
1  
@P.Brian.Mackey I don't think so. Questions 1 and 2 are about the overall design, not about the code. I think they're a better fit here. Part 3 is indeed fit for CR, but I recommend that user80195 wait for answers about the design here and ask for pointed advice on CR based on the feedback here. –  Gilles Feb 1 '13 at 20:38
    
What are you trying to accomplish calling a function in a shell script? –  MichaelT Feb 1 '13 at 20:52
1  
@user80195 - please consider putting some of my edits back in place as they help draw out the design aspects of your question which are on-topic for P.SE. After that edit, please flag your question for moderator review and re-opening. –  GlenH7 Feb 1 '13 at 21:08
2  
@user80195 You have received several comments and at least one answer that say, in varying terms, "Yes, this is a bad practice." Instead of pushing back against that feedback, you should accept that you have a reason why you're going against that advice and be done. –  Ross Patterson Feb 2 '13 at 15:13
show 8 more comments

2 Answers

up vote 1 down vote accepted

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.

share|improve this answer
    
I have already mentioned the reason for using functions.sh from a C binary at the beginning of the post. I will mention it again. I have a library of functions called functions.sh. I do not want to rewrite the library in C, therefore I am using it from a C program. 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). –  user80195 Feb 1 '13 at 23:32
1  
bash does have dynamically expandable arrays. Could you use those as a form of dynamic memory allocation? Your description of the overall problem, and your C program in particular, are so incomplete no one can really tell if calling shell functions via system() is a reasonable solution. All I can say that it sounds bad for the reasons I listed above. –  Charles E. Grant Feb 2 '13 at 1:45
    
Thanks Charles. Hope you realize my intention is not to push back against your advice, my intention is to get more feedback on technical drawbacks of my code. Dynamic memory allocation using arrays is very slow with bash. I'd like it to happen faster. But thanks for the feedback. –  user80195 Feb 2 '13 at 18:44
add comment

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.

share|improve this answer
    
Thanks Michael. Let me clarify some points. The logic is split but not entirely. I have a library in functions.sh that implements some decent logic in bash. But the only thing that I could not get done in bash was dynamic memory allocation. So I decided to use C. C code is not a dynamic data store in the sense that the o/p of last shell is used to determine what to run next. Basically what I do is: allocate memory in C, call specific functions from shell script in a specific order , check memory, end. functions.sh is a common library for other shell scripts as well. –  user80195 Feb 3 '13 at 20:05
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.