bugGNU Astronomy Utilities - Bugs: bug #65681, Remove all system calls within...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #65681: Remove all system calls within Gnuastro

Submitter:  Mohammad Akhlaghi <makhlaghi>
Submitted:  Fri 03 May 2024 07:46:11 PM UTC
   
 
Category:  All Gnuastro Severity:  3 - Normal
Item Group:  Crash Status:  Confirmed
Privacy:  Public Assigned to:  None
Open/Closed:  Open

Sun 19 May 2024 12:01:42 AM UTC, comment #4: 

In Commit bfc00b757b, the solution of the previous comment was generalized into an internal wrapper function 'gal_checkset_exec' and the PDF library now uses that. This should make complete removal of system() calls within Gnuastro easier ;-).

Mohammad Akhlaghi <makhlaghi>
Group administrator
Fri 17 May 2024 08:06:34 PM UTC, comment #3: 

Thanks to Sepideh Eskandarlou, I discovered that the 'execl' solution suggested in the previous comment will close the Gnuastro program that calls it! As a result, in the case of ConvertType calling Ghostscript, it couldn't delete the temporary EPS file.

A better solution (using fork+execl) is now implemented in Commit 89ffafd7fa.

Mohammad Akhlaghi <makhlaghi>
Group administrator
Fri 17 May 2024 10:47:40 AM UTC, comment #2: 

Even though the basic C program of the previous comment works, for some reason, within Gnuastro the problem remains: 'system' uses '/bin/sh'; even when PATH contains an 'sh' at another location with higher priority. We discovered this in a system with a faulty '/bin/sh' in the running environment.

A first implementation of replacing 'system()' with 'execl()' was therefore implemented in the ConvertType program in Commit 61fbd358a633; which worked successfully.

We should implement a similar process to remove all 'system()' calls within Gnuastro; once there is no more 'system()' calls within Gnuastro, we can close this bug.

Mohammad Akhlaghi <makhlaghi>
Group administrator
Sat 04 May 2024 10:39:29 AM UTC, comment #1: 

To better understand the behavior of C's 'system()' function, I wrote the following basic C program:


#include <stdio.h>
#include <stdlib.h>

int
main(void)
{
  system("echo $0; which $0");
  return EXIT_SUCCESS;
}


I then tested it with the following commands:


$ gcc system-test.c -o system-test
$ ./system-test
sh
/bin/sh
$ export PATH='/usr/bin:$PATH'
$ ./system-test
sh
/usr/bin/sh


So as explained in the manual, 'system()' does indeed respect the 'PATH' environment variable. The only thing that can't be changed is the 'sh'.

I also tried adding the 'system("echo $0; which $0");' within Query's source before the main 'system()' call and it behaves similar to above ('PATH' is respected).

This problem can so far only be reproduced in a Maneage'd project by Sepideh Eskandarlou (where '/bin/sh' is GNU Bash version 5.2.26 with Readline 8.2.010, and the custom shell within Query's environment uses Bash 5.1.8 and Readline 8.1.1).

Mohammad Akhlaghi <makhlaghi>
Group administrator
Fri 03 May 2024 07:46:11 PM UTC, original submission:  

Gnuastro query uses the 'system()' function of C to call the 'curl' command. On the other hand 'system()' uses '/bin/sh' to call the command that is given to it (even when '/bin/sh' is not in the PATH). As a result, when '/bin/sh' has a problem in the running environment, Gnuastro's Query will crash and there is no way for a user of Query to specify a different shell.

Having a look at the official documentation of 'system()' [1], it seems that 'system()' is just a wrapper over the 'execl()' function. So probably it is better to use the lower-level function and add a new option to Query and let the user specify the shell that Query should use for calling the 'curl' command.

I don't have time to look into this now! If anyone is interested, please post a comment here and look into it.

[1] https://pubs.opengroup.org/onlinepubs/007904975/functions/system.html

Mohammad Akhlaghi <makhlaghi>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by makhlaghi (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-05-17 makhlaghi CategoryQuery All Gnuastro
        SummaryQuery crashes because /bin/sh linking error Remove all system calls within Gnuastro

    Back to the top

    Powered by Savane 3.13-3cd0.
    Corresponding source code