@abh3 requested changes on this pull request.

Looks good, some nit-picking changes will seal the deal!

In docs/PreReleaseNotes.txt:

> @@ -31,6 +31,8 @@ Prerelease Notes
   **Commit: 0f79a38
   **[XrdMacaroons] Fix authentication when different tokens are used in the same TCP session**
   **Commit: 4410d56
+  **[XrdHttp] 500 Internal Server Error on unknown checksum algorithm**

That really is not a meaningful summary of the changes. How about "Follow RFC checksum requirements in XRootD http".

In src/XrdCl/XrdClUtils.hh:

> -        do
-        {
-          end = input.find( delimiter, start );
-
-          if( end != std::string::npos )
-            length = end - start;
-          else
-            length = input.length() - start;
-
-          if( length )
-            result.push_back( input.substr( start, length ) );
-
-          start = end + delimiter.size();
-        }
-        while( end != std::string::npos );
+        XrdOucUtils::splitString(result,input,delimiter);

Likely add a comment providing some history on this with a mention that at some point it would be good to collapse this change (i.e., eliminate the middle man call).

In src/XrdOuc/XrdOucUtils.hh:

> @@ -130,7 +131,36 @@ static bool PidFile(XrdSysError &eDest, const char *path);
 
 static int getModificationTime(const char * path, time_t & modificationTime);
 
-       XrdOucUtils() {}
-      ~XrdOucUtils() {}
+    //------------------------------------------------------------------------

It appears (I may be wrong) that this was inlined in the "hh" file. Could you move the actual implementation to the "cc" file to keep the "hh" file reasonably clean.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <xrootd/xrootd/pull/1890/review/1278870565@github.com>

Use REPLY-ALL to reply to list

To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1