Various enhancements for zopen tools #58
Conversation
IgorTodorovskiIBM
commented
Apr 22, 2024
•
IgorTodorovskiIBM
commented
- pthread_create-created threads do not set autocvt by default. This can be problematic if _BPXK_AUTOCVT is unset in the user environment. This PR adds a change to override pthread_create which sets AUTOCVT=ON in a similar way to the startup routine in zos.cc.
- adds netinet/in_systm.h is missing, affects honggfuzz and nmap
- adds getline and getdelim - affects honggfuzz, libssh2 and moreutils
- Adds _SC_NPROCESSORS_ONLN to sysconf - encountered too many times and rather than editing the application code, I extended sysconf to support _SC_NPROCESSORS_ONLN
- It also modifies the untagged read heuristic to remove the restriction on the min required bytes (currently set to 8 bytes). The reason for this is that there are <8 byte EBCDIC files such as /etc/syslog.pid and /etc/sshd.pid and based on user experience, untagged files which are small in size typically contain EBCDIC data and not binary data or ascii data so we should lean towards marking it as EBCDIC. Additionally, all z/OS Open Tools tag their data. Note, the __UNTAGGED_READ_MODE environment variable still takes precedence when set but most users do not set this environment variable.
- This will help resolve:
- nano untagged files zopencommunity/nanoport#11
- untagged issue? zopencommunity/vimport#52
- cat issue with untagged text zopencommunity/coreutilsport#68
- nano always writes out ascii zopencommunity/nanoport#5
- This will help resolve:
- Also changes chgfdccsid to not set the txtflag when ccsid = 0
- Adds more tests
src/zos-char-util.cc
Outdated
| if (ccsid == 1047 && len == cnt) { | ||
| if (no_tag_read_behaviour == __NO_TAG_READ_DEFAULT_WITHWARNING) { | ||
| if (name) { | ||
| len = strlen(name) + 1; |
There was a problem hiding this comment.
minor thing but i would typically have len be the strlen and then allocate the buffer to len+1 since you don't need to convert a 0 to a 0.
But perhaps just me.
src/zos-char-util.cc
Outdated
| } | ||
| } // cnt > 8 | ||
| fdcache.set_attribute(fd, 0x0000000000020000UL); |
There was a problem hiding this comment.
use a name for the magic number?
src/zos-io.cc
Outdated
| size_t pos = 0; | ||
|
|
||
| if (*lineptr == NULL || *n == 0) { | ||
| *n = 128; |
There was a problem hiding this comment.
why 128? and why 2 farther down? would those be good to have as buffer length and growth size macros (assuming that's what they are?)
src/zos-io.cc
Outdated
| int c; | ||
|
|
||
| if (buf == NULL || bufsize == 0) { | ||
| bufsize = 128; |
There was a problem hiding this comment.
same comment as above.
|
Added a readlink override to address zopencommunity/coreutilsport#74 and zopencommunity/gitport#122 |
src/zos-io.cc
Outdated
| *lineptr = (char *)malloc(*n); | ||
| if (*lineptr == NULL || *n == 0) { | ||
| *n = 120; | ||
| *lineptr = (char *)realloc(*lineptr, *n); |
There was a problem hiding this comment.
Calling realloc() this way is incorrect. If the memory allocation fails, then the original value of lineptr is lost, and the memory it pointed to is leaked. cppcheck complains about this, and suggests you use a temporary pointer to hold the return value, and then assign it to lineptr after validation.
https://stackoverflow.com/questions/21006707/proper-usage-of-realloc
- Override pthread_create to always set autocvt=on - Modify untagged heuristic to remove restriction on min required bytes (8) - Add tests
IgorTodorovskiIBM
force-pushed
the
zopen2
branch
from
5bbcd18 to
b0cd547
Compare
src/zos-instrumentation.cc
Outdated
| void *cur_dsa = __dsa(); | ||
|
|
||
| __iterate_stack_and_get(cur_dsa, &si); |
There was a problem hiding this comment.
Suggest checking return value from these two functions; if NULL, perhaps set the entry_name in the json entry to indicate that the stack could not be traversed. Same for the exit function below.
| size_t len = l_len - s_len + 1; | ||
| for (size_t i = 0; i < len; i++) | ||
| if (cs[0] == cl[i] && memcmp(cl + i, cs, s_len) == 0) | ||
| return (void *)(cl + i); |
There was a problem hiding this comment.
Extra indentation spaces here and in strverscmp() below.
| @@ -36,6 +36,10 @@ COMMAND /bin/as -mgoff -o ${CELQUOPT_OBJECT} ${CELQUOPT_SOURCE} | |||
| VERBATIM | |||
| ) | |||
|
|
|||
| if(${CMAKE_C_COMPILER} MATCHES clang) | |||
There was a problem hiding this comment.
Suggest checking if ! xlclang, since compiler can also be ibm-clang.
* Fix up product name & remove the c++ copied dir * more places to undo * Add Openxl v2.1 too
Co-authored-by: Sachin T <[email protected]>
