2
\$\begingroup\$

I've not written many shell scripts and cobbled this together from examples. The 'if' condition should check if this is already running and quite if so. The 'wget' call should trigger the php page and write any output to a txt file. The script will be launched by cron. Is this a reasonable approach?

#!/bin/sh
if ps -ef | grep -v grep | grep "dev.mydomain.biz/system_processdata.php" ; then
        exit 0
else
        wget -q -O /tmp/system_processimport.txt http://dev.mydomain.biz/system_processdata.php?a=37&b=61
        exit 0
fi
\$\endgroup\$
1
  • 1
    \$\begingroup\$ You might do better to look into pidof or psgrep rather than parsing the output of ps, to avoid potential false positives … ? \$\endgroup\$
    – BRPocock
    Dec 14, 2011 at 21:29

2 Answers 2

1
\$\begingroup\$

You should protect the URL with double quotes to prevent the shell from parsing the =, ?, and & as being special instead of just plain old characters.

Also, as a minor nitpick, you should use fgrep because grep will treat the . characters as being 'match any character' and not as a literal .. fgrep treats the whole string it's searching for (except for \n) as literal. But this is unlikely to cause you problems in practice.

The programmer in me shudders at this approach because so many things could potentially go wrong. In practice, they almost certainly won't. But I've learned to never, ever write programs as if the things I think should never happen never will. Because, in practice, they almost all do eventually.

For example, if you ever try to do two different fetches with different parameters at the same time, one will mysteriously not happen.

Also, if some random person happens to have put that URL on the command line for something, that will cause the script to not do anything until whatever command it is finishes.

As I said, these may or may not be serious problems. But I would be aware of them.

\$\endgroup\$
0
\$\begingroup\$

You should protect the URL with quotation marks since & is a control operator:

wget -q -O /tmp/out.txt "http://dev.mydomain.biz/system_processdata.php?a=37&b=61"

Furthermore, prevent concurrent calls on the server (PHP) side, it should be the server's responisibility.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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