Fix problems with spawnvpe and pthreads. #232
Conversation
elfpipe
commented
Dec 20, 2024
elfpipe
commented
- Fix problem with lingering pipes (spawnvpe).
- Fix problem with undertermined size of pthread_barrier_t (common.h/pthread.h).
…rmined size of pthread_barrier_t (common.h/pthread.h).
|
Code updated. |
library/c.lib_rev.h
Outdated
| #define VSTRING "clib4.library 1.4.1 (17.12.2024)\r\n" | ||
| #define VERSTAG "\0$VER: clib4.library 1.4.1-5d728de (17.12.2024)" | ||
| #define VSTRING "clib4.library 1.4.1 (20.12.2024)\r\n" | ||
| #define VERSTAG "\0$VER: clib4.library 1.4.1-5d728de (20.12.2024)" |
There was a problem hiding this comment.
These changes should not be in the repo. These are changed when we do a new release. If you be so kind to revert those changes.
There was a problem hiding this comment.
I didn't make those changes. There is some automatic feature autogenerating these.
| // DebugPrintF("[child :] Done."); | ||
| // FreeSignal(ed->childSignal); | ||
| // } | ||
| #define USE_CNPT 0 |
There was a problem hiding this comment.
If we are going to use this new definition, we probably need to document it at the README file. Can you please add some info about what this is, what does it enable and how to enable it if needed? It would be great if that was part of this PR.
There was a problem hiding this comment.
Maybe this is overkill. I am primarily keeping the version of the text using CreateNewProcTags() for research reasons. At present only SystemTags will enable full functioning in cmake. CNPT gives the strange side effect, that the child will have its parents name in argv[0] (at least when the child uses newlib, as in the case of (g)make).
I could be convinced to ditch the CNPT code, or just replace it with #if 0.
| snprintf(finalpath, finalpath_len, "%s %s", name, arg_string); | ||
| snprintf(processName, NAMELEN - 1, "Spawned Process #%d", __clib4->__children); | ||
| int full_command_len = strlen(name) + 1 + arg_string_len + 1; // '\0' | ||
| char *full_command = (char*)malloc(full_command_len); |
There was a problem hiding this comment.
I see that you renamed the finalpath to full_command but I do not see where this is freed. Maybe that was forgotten or we missed it in first place. Should we free that memory?
library/unistd/spawnvpe.c
Outdated
| closefh[2] = TRUE; | ||
| } | ||
|
|
||
| D(("(*)Calling SystemTags.\n")); |
There was a problem hiding this comment.
Maybe this line needs to get after #if USE_CNPT ... #else otherwise it will be confusing for users who check the debug and have the USE_CNPT enabled
There was a problem hiding this comment.
So, we might need one for CreateNewProcTags after line 297 and this one moved after line 336
library/unistd/spawnvpe.c
Outdated
| // } | ||
| /* * * * * | ||
| Note for future generations : CreateNewProc is not suited for running shell commands. | ||
| The only way to have a full shell environment (appart from using internal packet structures), |
4 participants
