V5.7.2 #20
Merged
V5.7.2 #20
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Seen on hppa Linux where EBADE is 160.
In function ‘int {anonymous}::initErrTable()’,
inlined from ‘void __static_initialization_and_destruction_0()’ at /<<PKGBUILDDIR>>/src/XrdSys/XrdSysE2T.cc:92:28,
inlined from ‘(static initializers for /<<PKGBUILDDIR>>/src/XrdSys/XrdSysE2T.cc)’ at /<<PKGBUILDDIR>>/src/XrdSys/XrdSysE2T.cc:128:1:
/<<PKGBUILDDIR>>/src/XrdSys/XrdSysE2T.cc:70:26: warning: array subscript 160 is above array bounds of ‘const char* [144]’ [-Warray-bounds=]
70 | if (Errno2String[EBADE]) {
| ~~~~~~~~~~~~~~~~~~^
/<<PKGBUILDDIR>>/src/XrdSys/XrdSysE2T.cc: In function ‘(static initializers for /<<PKGBUILDDIR>>/src/XrdSys/XrdSysE2T.cc)’:
/<<PKGBUILDDIR>>/src/XrdSys/XrdSysE2T.cc:48:28: note: while referencing ‘{anonymous}::Errno2String’
48 | const char* Errno2String[errSlots] = {0};
| ^~~~~~~~~~~~
In function ‘int {anonymous}::initErrTable()’,
inlined from ‘void __static_initialization_and_destruction_0()’ at /<<PKGBUILDDIR>>/src/XrdSys/XrdSysE2T.cc:92:28,
inlined from ‘(static initializers for /<<PKGBUILDDIR>>/src/XrdSys/XrdSysE2T.cc)’ at /<<PKGBUILDDIR>>/src/XrdSys/XrdSysE2T.cc:128:1:
/<<PKGBUILDDIR>>/src/XrdSys/XrdSysE2T.cc:74:22: warning: array subscript 160 is above array bounds of ‘const char* [144]’ [-Warray-bounds=]
74 | Errno2String[EBADE] = "authentication failed - possible invalid exchange";
| ~~~~~~~~~~~~~~~~~~^
/<<PKGBUILDDIR>>/src/XrdSys/XrdSysE2T.cc: In function ‘(static initializers for /<<PKGBUILDDIR>>/src/XrdSys/XrdSysE2T.cc)’:
/<<PKGBUILDDIR>>/src/XrdSys/XrdSysE2T.cc:48:28: note: while referencing ‘{anonymous}::Errno2String’
48 | const char* Errno2String[errSlots] = {0};
| ^~~~~~~~~~~~
The XrdSutCacheArg_t type is used to pass arguments of different types. The type used must be wide enough to fit the widest of the types passed. One of the types passed is time_t, which can be 64 bits wide also on 32-bit systems. So a long, which is 32 bits wide on a 32-bit system is not sufficient. This commit changes the type to long long. Fixes: xrootd#2272
Calling openssl rand with 0 as the argument is an error: $ openssl rand -base64 0 rand: Use -help for summary. $ echo $? 1
The files generated by doxygen whose file names contain the string unnamedN where N is a number are not generated using predictable names. This happens when running doxygen using multiple threads. Set NUM_PROC_THREADS=1 to work around this issue. See: doxygen/doxygen#11138
All supported platforms are now on C++17, so the pipeline API can be enabled everywhere. Issue: xrootd#2361
XRootD may be built as part of ROOT as a "builtin", so try to be nice by suppressing nvc++ warnings to avoid polluting ROOT's own build log. Issue: xrootd#2361
Causes problems due to unlimited parallel compilation with make. Better let this be controlled by setting CMAKE_BUILD_PARALLEL_LEVEL environment variable, which only works if --parallel option is not passed in the command line. https://cmake.org/cmake/help/latest/envvar/CMAKE_BUILD_PARALLEL_LEVEL.html Fixes: xrootd#2356
This directive was always set to true from the XrdTpc.cmake. We therefore don't need this directive. Removed it and removed the code that was never used because if it.
Also removed the redundant second transfer size request to improve efficiency. Fixes: xrootd#2354
These are very verbose, which may cause short test timeouts to trigger on slow systems. This change decreases the server log size from about 10k lines to about 1.8k for the noauth tests. Issue: xrootd#2336
If one does a GET /path/to/file.txt?authz=... the authz will be obfuscated in the logline "Parsing first line:"
Instead of invoking a filesystem 'stat', then opening the path, change the GET code to always try the 'open' first. If the open fails with an kXR_isDirectory (i.e., `EISDIR`), then the code falls back to doing a directory listing. At a minimum, this will halve the number of 'stat' requests the underlying storage will see as 'stat' is done twice per request if the requests are sent sequentially. At best, it'll eliminate nearly all stat calls under high concurrency given the OFS will collapse the open files into a single handle and some OSS's will cache the results of the first fstat. Co-authored-by: Fabrizio Furano <[email protected]>
Each time we process successfully from the redrive thread, we should decrement the status counter. Otherwise, after multiple requests, the attempt to `Run` from XrdHttp will fail. This was observed to cause failures in closing files 100% of the time when the open was delayed, causing failures in the XrdHttp response. This additionally adds logging on failures for submitting the run.
Report each error only once for each IO object, then report total counts for each error type once the IO object is detached. Previous repeating errors have been downgraded to Debug level.
…#2349 - For most parts, uses stat of the data file for reporting. - st_size is always set to the full size of the remote file. This is either obtained from cinfo file or from xattr variable created when the file is initially placed into cache (this is a new optimization). - st_atim is set to 0 when the file is not considered to be fully cached, as determined by the pfc.onlyifcached config settings.
Use POSIX regex functions instead of std::regex, as the latter is unusable due to the bug below. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86164
Between the call to stat and the open, the file size may change. First open the file, then use fstat with the file descriptor to ensure consistency.
First open then fstat, instead of stat then fopen, to guarantee that the information from stat and open are consistent with each other. Afterwards, convert file descriptor to FILE* with fdopen. Ensure also that the file is closed should an error occur.
Make sure that the information is consistent by first opening and subsequently using the same file descriptor for all operations. Since we needed to add an include as well, make sure to include our own headers first, then system headers.
The defaultID is created with defaultID = new XrdSecsssEnt(...); so it cannot be deallocated with free(defaultID). Since the destructor is private, we cannot call delete defaultID either. We must call the specific function for this, which is defaultID->Delete(), which in turn will call delete instead of free to deallocate the memory.
Using pointers in expressions that may cause overflow leads to undefined behavior. See also https://lwn.net/Articles/278137/
The variable jkey is already checked to be non-null a few lines above.
In line 707, there is already an if(!ifList || !(*ifList)) which makes the check in line 732 redundant.
The multiplication of int * int might be bigger than an int can hold, but still fits in a size_t. Convert before multiplying to ensure no overflow.
The multiplication is numq * xfrovhd, which if carried with ints can overflow before being converted to unsigned long long. Convert one of the operands to unsigned long long to avoid this.
From the man page for snprintf:
The functions snprintf() and vsnprintf() do not write more than size
bytes (including the terminating null byte ('\0')). If the output was
truncated due to this limit, then the return value is the number of
characters (excluding the terminating null byte) *which would have been
written to the final string if enough space had been available*. Thus,
a return value of size or more means that the output was truncated.
We therefore need to check if n > buffer size, and act accordingly.
The action to upload-artifact@v3 no longer works due to its reliance on node16, which has just hit EOL and been removed. Since upload-artifact@v4 also doesn't work on CentOS 7, the only option to keep builds running is to not upload results. https://github.blog/changelog/2024-04-16-deprecation-notice-v3-of-the-artifact-actions/
When moving to actions/checkout@v4, v1 was kept on Alpine Linux by mistake. The previous commit moved CentOS 7 properly to v1, and this commit fixes the problem in the Alpine Linux build. Fixes: 9096834
This reverts commit 50209b3. The fix is incorrect, as struct XrdSecsssRR_Data actually derives from XrdSecsssRR_DataHdr, which introduces some members in between such that prData.Data is not actually at the beginning of the struct, but at some non-zero offset (i.e. different address than &prData).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.