Print

Print


@abh3 requested changes on this pull request.


In src/XrdTpc/XrdTpcTPC.cc:

> @@ -779,13 +786,16 @@ int TPCHandler::ProcessPullReq(const std::string &resource, XrdHttpExtReq &req)
 }
 
 
-void TPCHandler::logTransferEvent(LogMask mask, const TPCLogRecord &rec,
+void TPCHandler::logTransferEvent(LogMask mask, TPCLogRecord &rec,

Changing this to non-const is bad and actually winds up not solving anything. See my comment later.


In src/XrdTpc/XrdTpcTPC.cc:

>          const std::string &event, const std::string &message)
 {
     if (!(m_log.getMsgMask() & mask)) {return;}
-
+    connDone.tv_sec = 0; connDone.tv_usec = 0;
+    gettimeofday( &connDone, 0 );
+    rec.eTOD = connDone;

No one uses eTOD outside of logging and it gets reset on every call. So, there is no reason to have it in TPCLogRecord and hence no need to make it non-const which is generally a bad idea. This is especially true because eTOD eally is misleading. This is actually the time the message was issued not when the request ended.


In src/XrdTpc/XrdTpcTPC.cc:

>      std::stringstream ss;
     ss << "event=" << event << ", local=" << rec.local << ", remote=" << rec.remote;
+    ss << ", stTime=" << rec.sTOD.tv_sec << ", enTime=" << rec.eTOD.tv_sec;

As per previous comment "enTime" is not really end time. It is merely the time the message was issued. So, I would suggest calling it what it really is.


In src/XrdTpc/XrdTpcTPC.hh:

> @@ -53,6 +53,8 @@ private:
                          streams( 1 ),
                          bytes_transferred( -1 )
         {
+          sTOD.tv_sec = 0; sTOD.tv_usec = 0;
+          eTOD.tv_sec = 0; eTOD.tv_usec = 0;

As per my previous comments, sTOD makes sense but eTOD as it is being haled does not.


Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.Message ID: <xrootd/xrootd/pull/1529/review/819633463@github.com>

[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/xrootd/xrootd/pull/1529#pullrequestreview-819633463", "url": "https://github.com/xrootd/xrootd/pull/1529#pullrequestreview-819633463", "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