support DB_PROGRAM to be absolute, relative or in PATH#4013
support DB_PROGRAM to be absolute, relative or in PATH#4013TurBoss wants to merge 4 commits intoLinuxCNC:masterfrom
Conversation
| char* saveptr; | ||
| char* token = strtok_r(progname_plus_args, " ", &saveptr); | ||
| int n; | ||
| char* fork_argv[PATH_MAX] = {0}; |
There was a problem hiding this comment.
Why are you allocating PATH_MAX pointers to char? I think that PATH_MAX is normally about 4096 and you are allocating that many pointers. Should use std::vector.
See also other comment about this.
There was a problem hiding this comment.
reduced that to 10, was a mistake thanks!
| if (db_childname[0] != '/') { // prog path not absolute | ||
| if(strchr(db_childname, '/') != NULL){ // prog path is relative | ||
| strncpy(prog_path, db_childname, PATH_MAX); | ||
| } | ||
| else { // just prog name, search in the actual directory or PATH | ||
| envpath = getenv( (char*)"PATH"); |
There was a problem hiding this comment.
...and the rest...
I think this needs to be redesigned using std::string and std::vector<std::string>. There is no point nowadays to use C style (more error prone) string handling in a C++ source file. It was already outdated in 2021 when written and added. Now, when you try to fix it, it should be modernized to remove the standard pitfalls.
There was a problem hiding this comment.
sorry I'm not much familiar with c++
|
Some more observations while reading this code... When you change the code to use a path search, why then not make it easier for yourself and run it via a shell -c exec (not a call to system)? Then the shell can deal with all the intricacies of argument splitting and path searches? The code will crash and burn when the maximum number of arguments are parsed because the call to execv() must terminate the arg list with a NULL. In effect, this is a off-by-one error. The code also spits out stuff using fprintf() and not the usual rcs_print stuff. This happens a lot of places (not only in the tooldata stuff). Other things in the code: The code gets debug variables from the environment, where it should be reading those from the INI-file as the central place for configuration. The SIGCHLD handler waits for all processes that may be a zombi (that is good), however, it then only checks the last of the zombi's status. What about the others? Probably a lot more if I start looking deeper. Enough for now. |
|
BTW, using a shell -c is also a very tricky suggestion requiring a lot of escaping. |
2 participants
Hello
This commit allows DB_PROGRAM to handle absolute, relative path or binaries in env PATH
closes #3982
Thanks
Edit: also check if path/file exists