Skip to content

Commit dff80fd

Browse files
committed
Minor fixes from code review
1 parent b14c70b commit dff80fd

File tree

7 files changed

+30
-26
lines changed

7 files changed

+30
-26
lines changed

src/GlobusDirectory.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ void GlobusDirectory::Reset() {
5454
int GlobusDirectory::ListGlobusDir() {
5555
m_log.Log(XrdHTTPServer::Debug, "GlobusDirectory::ListGlobusDir",
5656
"Listing directory:", m_object.c_str());
57-
HTTPDownload listCommand(m_fs->getLsUrl(), m_object, m_log,
58-
m_fs->getTransferToken());
57+
HTTPDownload listCommand(m_fs.getLsUrl(), m_object, m_log,
58+
m_fs.getTransferToken());
5959

6060
if (!listCommand.SendRequest(0, 0)) {
6161
return HTTPRequest::HandleHTTPError(
@@ -118,7 +118,7 @@ int GlobusDirectory::Opendir(const char *path, XrdOucEnv &env) {
118118
realPath = realPath + "/";
119119
}
120120

121-
std::string storagePrefix = m_fs->getStoragePrefix();
121+
std::string storagePrefix = m_fs.getStoragePrefix();
122122
std::string object;
123123

124124
if (realPath.find(storagePrefix) == 0) {
@@ -257,4 +257,4 @@ int GlobusDirectory::Close(long long *retsz) {
257257
}
258258
Reset();
259259
return XrdOssOK;
260-
}
260+
}

src/GlobusDirectory.hh

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ struct GlobusObjectInfo {
3333

3434
class GlobusDirectory : public HTTPDirectory {
3535
public:
36-
GlobusDirectory(XrdSysError &log, const GlobusFileSystem *fs)
36+
GlobusDirectory(XrdSysError &log, const GlobusFileSystem &fs)
3737
: HTTPDirectory(log), m_fs(fs) {}
3838

3939
virtual ~GlobusDirectory() {}
@@ -56,6 +56,6 @@ class GlobusDirectory : public HTTPDirectory {
5656
std::vector<GlobusObjectInfo> m_directories;
5757
std::string m_prefix;
5858
std::string m_object;
59-
const GlobusFileSystem *m_fs;
59+
const GlobusFileSystem &m_fs;
6060
struct stat *m_stat_buf{nullptr};
61-
};
61+
};

src/GlobusFile.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@
3939
extern "C" {
4040

4141
/*
42-
This function is called when we are wrapping something.
42+
The GlobusFileSystem adds Globus-specific functionality on top of the
43+
HTTPFileSystem. Rather than re-implement or re-compile things, for now,
44+
we simply assume we wrap on top.
4345
*/
4446
XrdOss *XrdOssAddStorageSystem2(XrdOss *curr_oss, XrdSysLogger *logger,
4547
const char *config_fn, const char *parms,
@@ -58,4 +60,4 @@ XrdOss *XrdOssAddStorageSystem2(XrdOss *curr_oss, XrdSysLogger *logger,
5860

5961
} // end extern "C"
6062

61-
XrdVERSIONINFO(XrdOssAddStorageSystem2, globus);
63+
XrdVERSIONINFO(XrdOssAddStorageSystem2, globus);

src/GlobusFile.hh

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@
2323

2424
class GlobusFile final : public XrdOssWrapDF {
2525
public:
26-
GlobusFile(std::unique_ptr<XrdOssDF> wrapped, XrdSysError &log,
27-
GlobusFileSystem *fs)
26+
GlobusFile(std::unique_ptr<XrdOssDF> wrapped, XrdSysError &log)
2827
: XrdOssWrapDF(*wrapped), m_wrapped(std::move(wrapped)) {}
2928

3029
virtual ~GlobusFile() {}

src/GlobusFileSystem.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,12 @@ bool GlobusFileSystem::Config(XrdSysLogger *lp, const char *configfn) {
137137
}
138138

139139
XrdOssDF *GlobusFileSystem::newDir(const char *user) {
140-
return new GlobusDirectory(m_log, this);
140+
return new GlobusDirectory(m_log, *this);
141141
}
142142

143143
XrdOssDF *GlobusFileSystem::newFile(const char *user) {
144144
std::unique_ptr<XrdOssDF> wrapped(wrapPI.newFile(user));
145-
return new GlobusFile(std::move(wrapped), m_log, this);
145+
return new GlobusFile(std::move(wrapped), m_log);
146146
}
147147

148148
int GlobusFileSystem::Stat(const char *path, struct stat *buff, int opts,

src/HTTPCommands.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ bool HTTPRequest::SendHTTPRequest(const std::string &payload) {
145145
m_log.Log(LogMask::Debug, "HTTPRequest::SendHTTPRequest",
146146
"Sending request");
147147

148-
return sendPreparedRequest(hostUrl, payload, payload.size(), final);
148+
return sendPreparedRequest(hostUrl, payload, payload.size(), true);
149149
}
150150

151151
static void dump(XrdSysError *log, const char *text, unsigned char *ptr,

src/HTTPCommands.hh

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -96,18 +96,21 @@ class HTTPRequest {
9696
return m_timeout_duration;
9797
}
9898

99-
// Handle HTTP request errors and convert them to appropriate POSIX error codes.
100-
// This function can be used anywhere HTTP requests are made to provide
101-
// consistent error handling.
99+
// Handle HTTP request errors and convert them to appropriate POSIX error
100+
// codes. This function can be used anywhere HTTP requests are made to
101+
// provide consistent error handling.
102102
//
103103
// - request: The HTTPRequest object that was used for the request
104104
// - log: The logger instance for error reporting
105-
// - operation: A string describing the operation being performed (for logging)
105+
// - operation: A string describing the operation being performed (for
106+
// logging)
106107
// - context: Additional context information (for logging)
107108
//
108-
// Returns: A POSIX error code (-ENOENT, -EIO, -EPERM, etc.) or 0 if no error
109+
// Returns: A POSIX error code (-ENOENT, -EIO, -EPERM, etc.) or 0 if no
110+
// error
109111
static int HandleHTTPError(const HTTPRequest &request, XrdSysError &log,
110-
const char *operation, const char *context = nullptr);
112+
const char *operation,
113+
const char *context = nullptr);
111114

112115
protected:
113116
// Send the request to the HTTP server.
@@ -299,7 +302,7 @@ class HTTPRequest {
299302
static std::chrono::steady_clock::duration m_timeout_duration;
300303
};
301304

302-
class HTTPUpload : public HTTPRequest {
305+
class HTTPUpload final : public HTTPRequest {
303306
public:
304307
HTTPUpload(const std::string &h, const std::string &o, XrdSysError &log,
305308
const TokenFile *token)
@@ -332,7 +335,7 @@ class HTTPUpload : public HTTPRequest {
332335
std::string path;
333336
};
334337

335-
class HTTPDownload : public HTTPRequest {
338+
class HTTPDownload final : public HTTPRequest {
336339
public:
337340
HTTPDownload(const std::string &h, const std::string &o, XrdSysError &log,
338341
const TokenFile *token)
@@ -348,7 +351,7 @@ class HTTPDownload : public HTTPRequest {
348351
std::string object;
349352
};
350353

351-
class HTTPHead : public HTTPRequest {
354+
class HTTPHead final : public HTTPRequest {
352355
public:
353356
HTTPHead(const std::string &h, const std::string &o, XrdSysError &log,
354357
const TokenFile *token)
@@ -364,10 +367,10 @@ class HTTPHead : public HTTPRequest {
364367
std::string object;
365368
};
366369

367-
class HTTPDelete : public HTTPRequest {
370+
class HTTPDelete final : public HTTPRequest {
368371
public:
369372
HTTPDelete(const std::string &h, const std::string &o, XrdSysError &log,
370-
const TokenFile *token)
373+
const TokenFile *token)
371374
: HTTPRequest(h, log, token), object(o) {
372375
hostUrl = hostUrl + "/" + object;
373376
}
@@ -378,4 +381,4 @@ class HTTPDelete : public HTTPRequest {
378381

379382
protected:
380383
std::string object;
381-
};
384+
};

0 commit comments

Comments
 (0)