Thread (18 messages) flat view 18 messages, 5 authors, 2016-06-15

Re: [PATCH/WIP 1/8] wrapper: implement xopen()

From: Torsten Bögershausen <hidden>
Date: 2016-06-15 23:05:01

On 2015-05-27 15.33, Paul Tan wrote:
quoted hunk ↗ jump to hunk
A common usage pattern of open() is to check if it was successful, and
die() if it was not:

	int fd = open(path, O_WRONLY | O_CREAT, 0777);
	if (fd < 0)
		die_errno(_("Could not open '%s' for writing."), path);

Implement a wrapper function xopen() that does the above so that we can
save a few lines of code, and make the die() messages consistent.

Signed-off-by: Paul Tan <redacted>
---
 git-compat-util.h |  1 +
 wrapper.c         | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index 17584ad..9745962 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -718,6 +718,7 @@ extern char *xstrndup(const char *str, size_t len);
 extern void *xrealloc(void *ptr, size_t size);
 extern void *xcalloc(size_t nmemb, size_t size);
 extern void *xmmap(void *start, size_t length, int prot, int flags, int fd, off_t offset);
+extern int xopen(const char *path, int flags, mode_t mode);
 extern ssize_t xread(int fd, void *buf, size_t len);
 extern ssize_t xwrite(int fd, const void *buf, size_t len);
 extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
diff --git a/wrapper.c b/wrapper.c
index c1a663f..971665a 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -189,6 +189,24 @@ void *xcalloc(size_t nmemb, size_t size)
 # endif
 #endif
 
The original open can take 2 or 3 parameters, how about this:
int xopen(const char *path, int oflag, ... )
{
        va_list params;
        int mode;
        int fd;

        va_start(params, oflag);
        mode = va_arg(params, int);
        va_end(params);

        fd = open(path, oflag, mode);

+/**
+ * xopen() is the same as open(), but it die()s if the open() fails.
+ */
+int xopen(const char *path, int flags, mode_t mode)
+{
+	int fd;
+
+	assert(path);
+	fd = open(path, flags, mode);
+	if (fd < 0) {
+		if ((flags & O_WRONLY) || (flags & O_RDWR))
+			die_errno(_("could not open '%s' for writing"), path);
This is only partly true:
it could be either "writing" or "read write".
I don't know if the info "for reading" or "for writing" is needed/helpful at all,
or if a simple "could not open" would be enough.


Another thing:
should we handle EINTR ?
(Somewhere in the back of my head I remember that some OS
 returned EINTR when handling some foreign file system
 Mac OS / NTFS ?)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help