Print

Print


@amadio commented on this pull request.


In src/XrdHttp/XrdHttpChecksumHandler.cc:

> +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.


In src/XrdHttp/XrdHttpChecksumHandler.hh:

> +// 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, view it on GitHub, or unsubscribe.
You are receiving this because you commented.Message ID: <xrootd/xrootd/pull/1950/review/1341190826@github.com>

[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/xrootd/xrootd/pull/1950#pullrequestreview-1341190826", "url": "https://github.com/xrootd/xrootd/pull/1950#pullrequestreview-1341190826", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://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