From Bugzilla Helper: User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.2) Gecko/20030716 Description of problem: startFilesystems() and stopFilesystems() shell functions There's a bug in the error handling code relating to the creation of temporary files: 1. If the "mktemp" should fail, as determined by testing whether the shell variable "TMPFILE" is empty, the "logandPrint" function is called to log the error. If you look carefully, you'll notice that there's a missing backslash before the newline and thus the command is not continued. In the event of an error, this would likely prove very confusing. 2. If you look closer still at the assignment of the "TMPFILE" variable (see below), you'll notice that the "ret_val" variable is assigned as part of the same sub-shell that first calls the "mktemp" command. As a result, the value won't be accessible to the calling shell. At least, that's what I understand from my shell programming. typeset TMPFILE=$(mktemp $TMPFILE_STR; ret_val=$?) Version-Release number of selected component (if applicable): clumanager-1.0.25-1 How reproducible: Always Steps to Reproduce: N/A Additional info: A quick "grep mktemp *" inside the /usr/share/cluster/services directory would indicate that this same code might be present in other include files.
Created attachment 97298 [details] Patch against 1.0.25 to fix problem Here's a patch against 1.0.25 which fixes the problem. It also appears in svclib_device.
Thanks for the patch! However, I don't believe the patch addresses the second point in my original bug report. I think you'll find that the code snippet below does not set the value of the "ret_val" variable in the calling shell for the reason already explained. typeset TMPFILE=$(mktemp $TMPFILE_STR; ret_val=$?) Try it yourself. In fact, I don't think it's possible to do what you're trying to do. IMHO, it's better to code this in a more straightforward way a la following: typeset TMPFILE if { ! mktemp $TMPFILE_STR; } then etc... fi Then again, given "mktemp" only returns 0 or 1, you might as well just test whether the string is empty and just leave out the "ret_val" bit all together. The code as it is is misleading.
Right, sorry. You're right, the following is not possible: typeset TMPFILE=$(mktemp $TEMPFILE_STR; ret_val=$?) I'm not sure how the author came up with that line. In general, you're more likely to run out of inodes on your file system, in which case, 'mktemp' failing is the least of your worries. ;) I can change the code to fix it so that it will correctly do the assignments. It's not elegant (e.g. still misleading), but it will ensure that the log message actually gets out in the unlikely case that mktemp fails, and has minimal impact on the existing code.
Created attachment 97336 [details] Patch to fix both mktemp / backslash weirdness Patch against 1.0.25 which fixes both of the potential problems.
Sorry to be a pedant...but it still doesn't work. Here's why: [peterg@aolinux tmp]$ ( cat /etc/redhat-release; echo $SHELL; typeset -i ret_val; typeset SOMEVAR=$(/bin/false); ret_val=$?; echo $ret_val; ) Red Hat Linux release 8.0 (Psyche) /bin/bash 0 [peterg@aolinux tmp]$ ( cat /etc/redhat-release; echo $SHELL; typeset -i ret_val; typeset SOMEVAR=$(/bin/true); ret_val=$?; echo $ret_val; ) Red Hat Linux release 8.0 (Psyche) /bin/bash 0 [peterg@aolinux tmp]$ ( cat /etc/redhat-release; echo $SHELL; typeset -i ret_val; typeset SOMEVAR=$(/bin/anyolthing); ret_val=$?; echo $ret_val; ) Red Hat Linux release 8.0 (Psyche) /bin/bash bash: /bin/anyolthing: No such file or directory 0 [peterg@aolinux tmp]$ ( cat /etc/redhat-release; echo $SHELL; typeset -i ret_val; typeset -abc SOMEVAR=$(/bin/anyolthing); ret_val=$?; echo $ret_val; ) Red Hat Linux release 8.0 (Psyche) /bin/bash bash: /bin/anyolthing: No such file or directory bash: typeset: -b: invalid option typeset: usage: typeset [-afFirtx] [-p] name[=value] ... 2 In other words, it's the exit value of the "typeset" command (which will always be true!) that is being assigned to "ret_val" -- not the "mktemp". IMHO, it's better to omit the "ret_val" all together as it doesn't tell you anything. Ultimately, it's whether the value is non-empty that counts. If you really want to be paranoid you might also test whether the file exists and is a regular file. That is: if ! [[ -n "$tmpfile" && -f "$tmpfile" ]]; then logAndPrint ... fi
The typeset lines and assignments are on different lines with the patch applied. The patch makes the code behave in the following manner, to use the above examples: typeset -i ret_val typeset SOMEVAR SOMEVAR=$(/bin/false); ret_val=$? The above causes the correct behavior. Consequently, what the examples above should have done is: ( cat /etc/redhat-release; echo $SHELL; typeset -i ret_val; typeset SOMEVAR; SOMEVAR=$(/bin/false); ret_val=$?; echo $ret_val; ) This works properly: [lhh@eagle lhh]$ ( cat /etc/redhat-release; echo $SHELL; typeset -i ret_val; typeset SOMEVAR; SOMEVAR=$(/bin/false); ret_val=$?; echo $ret_val; ) Red Hat Enterprise Linux WS release 3 (Taroon) /bin/bash 1 [lhh@eagle lhh]$
I should've looked more carefully at the patch! Thanks for the explanation. Case closed!
Waiting for erratum / update to close; leaving as 'MODIFIED' for now.
Note: This also affects RHEL3
Unable to test this, but the opener seemed to be fine with it. Closing as errata.