Print

Print


@amadio commented on this pull request.



> +std::map<std::string,XrdHttpChecksumHandlerImpl::XrdHttpChecksumPtr> XrdHttpChecksumHandlerImpl::XROOTD_TO_HTTP_CKSUMS;
+XrdHttpChecksumHandlerImpl::XrdHttpChecksumPtr XrdHttpChecksumHandlerImpl::NONE_CONFIGURED_CKSUM = std::make_shared<XrdHttpChecksum>("none_configured", "none_configured", false);
+
+void XrdHttpChecksumHandlerImpl::initializeCksumsMaps() {
+    addChecksumToMaps(std::make_shared<XrdHttpChecksum>("md5","md5",true));
+    addChecksumToMaps(std::make_shared<XrdHttpChecksum>("adler32","adler32",false));
+    addChecksumToMaps(std::make_shared<XrdHttpChecksum>("sha1","sha",true));
+    addChecksumToMaps(std::make_shared<XrdHttpChecksum>("sha256","sha-256",true));
+    addChecksumToMaps(std::make_shared<XrdHttpChecksum>("sha512","sha-512",true));
+    addChecksumToMaps(std::make_shared<XrdHttpChecksum>("cksum","UNIXcksum",false));
+    addChecksumToMaps(std::make_shared<XrdHttpChecksum>("crc32","crc32",false));
+    addChecksumToMaps(std::make_shared<XrdHttpChecksum>("crc32c","crc32c",true));
+}

I wonder if we can manage to avoid the `std::map` and `std::make_shared` here. If the list is defined at compile time, maybe a simple array of structs with two strings and a bool is enough? That would go in the data section rather than the heap.

> +// XRootD is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with XRootD.  If not, see <http://www.gnu.org/licenses/>.
+//------------------------------------------------------------------------------
+#ifndef XROOTD_XRDHTTPCHECKSUMHANDLER_HH
+#define XROOTD_XRDHTTPCHECKSUMHANDLER_HH
+
+#include <string>
+#include <map>
+#include <vector>
+#include "XrdHttpChecksum.hh"
+#include <memory>

Should we try to include our headers first? This order of includes is somewhat strange.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/xrootd/xrootd/pull/1950#pullrequestreview-1341190826
You are receiving this because you commented.

Message ID: <[log in to unmask]>

########################################################################
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