You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jp...@apache.org on 2014/04/10 23:43:32 UTC
git commit: TS-898: ensure cache span probe closes file descriptors
Repository: trafficserver
Updated Branches:
refs/heads/master 17579ee72 -> e503ce048
TS-898: ensure cache span probe closes file descriptors
Add a new helper class xfd (by analogy to xptr). This ensures that
the opened file descriptor is always closed in every error path.
Coverity #1196462
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/e503ce04
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/e503ce04
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/e503ce04
Branch: refs/heads/master
Commit: e503ce048bb5fcab84b251ebcc31efa972d67710
Parents: 17579ee
Author: James Peach <jp...@apache.org>
Authored: Sun Apr 6 11:25:11 2014 -0700
Committer: James Peach <jp...@apache.org>
Committed: Thu Apr 10 14:43:22 2014 -0700
----------------------------------------------------------------------
CHANGES | 2 ++
iocore/cache/Store.cc | 27 +++++++++-----------
iocore/eventsystem/I_SocketManager.h | 42 +++++++++++++++++++++++++++++++
3 files changed, 56 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e503ce04/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 3f113bc..2115ad8 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
-*- coding: utf-8 -*-
Changes with Apache Traffic Server 5.0.0
+ *) [TS-898] Ensure the cache span probe always closes file descriptors.
+
*) [TS-2706] Replace the tcp_info plugin config file with options.
*) [TS-2691] Fix how we count redirect retries in the core and APIs.
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e503ce04/iocore/cache/Store.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/Store.cc b/iocore/cache/Store.cc
index 2e79958..46c346c 100644
--- a/iocore/cache/Store.cc
+++ b/iocore/cache/Store.cc
@@ -496,16 +496,15 @@ Span::init(char *an, int64_t size)
return "error stat of file";
}
- int fd = socketManager.open(n, O_RDONLY);
- if (fd < 0) {
- Warning("unable to open '%s': %d, %s", n, fd, strerror(errno));
+ xfd fd(socketManager.open(n, O_RDONLY));
+ if (!fd) {
+ Warning("unable to open '%s': %s", n, strerror(errno));
return "unable to open";
}
struct statvfs fs;
if ((ret = fstatvfs(fd, &fs)) < 0) {
Warning("unable to statvfs '%s': %d %d, %s", n, ret, errno, strerror(errno));
- socketManager.close(fd);
return "unable to statvfs";
}
@@ -569,7 +568,6 @@ Span::init(char *an, int64_t size)
Debug("cache_init", "Span::init - %s hw_sector_size = %d size = %" PRId64 ", blocks = %" PRId64 ", disk_id = %d, file_pathname = %d", pathname, hw_sector_size, size, blocks, disk_id, file_pathname);
Lfail:
- socketManager.close(fd);
return err;
}
@@ -589,9 +587,9 @@ Span::init(char *filename, int64_t size)
//
is_mmapable_internal = true;
- int fd = socketManager.open(filename, O_RDONLY);
- if (fd < 0) {
- Warning("unable to open '%s': %d, %s", filename, fd, strerror(errno));
+ xfd fd(socketManager.open(filename, O_RDONLY));
+ if (!fd) {
+ Warning("unable to open '%s': %s", filename, strerror(errno));
return "unable to open";
}
@@ -657,7 +655,6 @@ Span::init(char *filename, int64_t size)
Debug("cache_init", "Span::init - %s hw_sector_size = %d size = %" PRId64 ", blocks = %" PRId64 ", disk_id = %d, file_pathname = %d", filename, hw_sector_size, size, blocks, disk_id, file_pathname);
Lfail:
- socketManager.close(fd);
return err;
}
#endif
@@ -672,9 +669,10 @@ Lfail:
const char *
Span::init(char *filename, int64_t size)
{
- int devnum = 0, fd, arg = 0;
+ int devnum = 0, arg = 0;
int ret = 0, is_disk = 0;
u_int64_t heads, sectors, cylinders, adjusted_sec;
+ xfd fd;
/* Fetch file type */
struct stat stat_buf;
@@ -705,11 +703,12 @@ Span::init(char *filename, int64_t size)
break;
}
- if ((fd = socketManager.open(filename, O_RDONLY)) < 0) {
- Warning("unable to open '%s': %d, %s", filename, fd, strerror(errno));
+ fd = socketManager.open(filename, O_RDONLY);
+ if (!fd) {
+ Warning("unable to open '%s': %s", filename, strerror(errno));
return "unable to open";
}
- Debug("cache_init", "Span::init - socketManager.open(\"%s\", O_RDONLY) = %d", filename, fd);
+ Debug("cache_init", "Span::init - socketManager.open(\"%s\", O_RDONLY) = %d", filename, (int)fd);
adjusted_sec = 1;
#ifdef BLKPBSZGET
@@ -822,8 +821,6 @@ Span::init(char *filename, int64_t size)
disk_id = devnum;
- socketManager.close(fd);
-
return NULL;
}
#endif
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e503ce04/iocore/eventsystem/I_SocketManager.h
----------------------------------------------------------------------
diff --git a/iocore/eventsystem/I_SocketManager.h b/iocore/eventsystem/I_SocketManager.h
index d294a5c..6d3dc07 100644
--- a/iocore/eventsystem/I_SocketManager.h
+++ b/iocore/eventsystem/I_SocketManager.h
@@ -134,4 +134,46 @@ private:
extern SocketManager socketManager;
+struct xfd {
+
+ xfd() : m_fd(-1) {
+ }
+
+ explicit xfd(int _fd) : m_fd(_fd) {
+ }
+
+ ~xfd() {
+ if (this->m_fd != -1) {
+ socketManager.close(this->m_fd);
+ }
+ }
+
+ /// Auto convert to a raw file descriptor.
+ operator int() const { return m_fd; }
+
+ /// Boolean operator. Returns true if we have a valid file descriptor.
+ operator bool() const { return m_fd != -1; }
+
+ xfd& operator=(int fd) {
+ if (this->m_fd != -1) {
+ socketManager.close(this->m_fd);
+ }
+
+ this->m_fd = fd;
+ return *this;
+ }
+
+ int release() {
+ int tmp = this->m_fd;
+ this->m_fd = -1;
+ return tmp;
+ }
+
+ private:
+ int m_fd;
+
+ xfd(xfd const&); // disabled
+ xfd& operator=(xfd const&); // disabled
+};
+
#endif /*_SocketManager_h_*/