{"thread":{"id":"56377","subject":"[PATCH 1/2] xopen: explicitly report creation failures","startedAt":"2021-08-25T20:14:16Z","lastAt":"2021-08-26T16:49:33Z","messageCount":5,"participants":["René Scharfe","Carlo Arenas","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"433765","messageId":"6a5c3e8e-0216-8b63-38fa-b7b19331d752@web.de","threadId":"56377","inReplyTo":null,"subject":"[PATCH 1/2] xopen: explicitly report creation failures","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2021-08-25T20:14:09Z","receivedAt":"2021-08-25T20:14:16Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"If the flags O_CREAT and O_EXCL are both given then open(2) is supposed\nto create the file and error out if it already exists.  The error\nmessage in that case looks like this:\n\n\tfatal: could not open 'foo' for writing: File exists\n\nWithout further context this is confusing: Why should the existence of\nthe file pose a problem?  Isn't that a requirement for writing to it?\n\nAdd a more specific error message for that case to tell the user that we\nactually don't expect the file to preexist, so the example becomes:\n\n\tfatal: unable to create 'foo': File exists\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n wrapper.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/wrapper.c b/wrapper.c\nindex 563ad590df..7c6586af32 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -193,7 +193,9 @@ int xopen(const char *path, int oflag, ...)\n \t\tif (errno == EINTR)\n \t\t\tcontinue;\n\n-\t\tif ((oflag & O_RDWR) == O_RDWR)\n+\t\tif ((oflag & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL))\n+\t\t\tdie_errno(_(\"unable to create '%s'\"), path);\n+\t\telse if ((oflag & O_RDWR) == O_RDWR)\n \t\t\tdie_errno(_(\"could not open '%s' for reading and writing\"), path);\n \t\telse if ((oflag & O_WRONLY) == O_WRONLY)\n \t\t\tdie_errno(_(\"could not open '%s' for writing\"), path);\n--\n2.33.0\n"},{"id":"433766","messageId":"5f319f04-e4f3-678d-f35f-88eef2ce3e5f@web.de","threadId":"56377","inReplyTo":"6a5c3e8e-0216-8b63-38fa-b7b19331d752@web.de","subject":"[PATCH 2/2] use xopen() to handle fatal open(2) failures","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2021-08-25T20:16:46Z","receivedAt":"2021-08-25T20:16:54Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Add and apply a semantic patch for using xopen() instead of calling\nopen(2) and die() or die_errno() explicitly.  This makes the error\nmessages more consistent and shortens the code.\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n builtin/add.c                  |  4 +---\n builtin/archive.c              |  4 +---\n builtin/bugreport.c            |  5 +----\n builtin/commit-tree.c          |  4 +---\n builtin/hash-object.c          |  4 +---\n builtin/index-pack.c           |  8 ++------\n builtin/mailsplit.c            |  4 +---\n builtin/merge.c                |  4 +---\n builtin/notes.c                |  4 +---\n builtin/tag.c                  |  4 +---\n builtin/update-index.c         |  4 +---\n contrib/coccinelle/xopen.cocci | 16 ++++++++++++++++\n csum-file.c                    |  8 ++------\n pack-write.c                   |  8 ++------\n run-command.c                  |  4 +---\n 15 files changed, 33 insertions(+), 52 deletions(-)\n create mode 100644 contrib/coccinelle/xopen.cocci\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 17528e8f92..8ec5ad9f26 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -319,9 +319,7 @@ static int edit_patch(int argc, const char **argv, const char *prefix)\n \trev.diffopt.output_format = DIFF_FORMAT_PATCH;\n \trev.diffopt.use_color = 0;\n \trev.diffopt.flags.ignore_dirty_submodules = 1;\n-\tout = open(file, O_CREAT | O_WRONLY | O_TRUNC, 0666);\n-\tif (out < 0)\n-\t\tdie(_(\"Could not open '%s' for writing.\"), file);\n+\tout = xopen(file, O_CREAT | O_WRONLY | O_TRUNC, 0666);\n \trev.diffopt.file = xfdopen(out, \"w\");\n \trev.diffopt.close_file = 1;\n \tif (run_diff_files(&rev, 0))\ndiff --git a/builtin/archive.c b/builtin/archive.c\nindex 45d11669aa..7176b041b6 100644\n--- a/builtin/archive.c\n+++ b/builtin/archive.c\n@@ -12,9 +12,7 @@\n\n static void create_output_file(const char *output_file)\n {\n-\tint output_fd = open(output_file, O_CREAT | O_WRONLY | O_TRUNC, 0666);\n-\tif (output_fd < 0)\n-\t\tdie_errno(_(\"could not create archive file '%s'\"), output_file);\n+\tint output_fd = xopen(output_file, O_CREAT | O_WRONLY | O_TRUNC, 0666);\n \tif (output_fd != 1) {\n \t\tif (dup2(output_fd, 1) < 0)\n \t\t\tdie_errno(_(\"could not redirect output\"));\ndiff --git a/builtin/bugreport.c b/builtin/bugreport.c\nindex 9915a5841d..06ed10dc92 100644\n--- a/builtin/bugreport.c\n+++ b/builtin/bugreport.c\n@@ -171,10 +171,7 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)\n \tget_populated_hooks(&buffer, !startup_info->have_repository);\n\n \t/* fopen doesn't offer us an O_EXCL alternative, except with glibc. */\n-\treport = open(report_path.buf, O_CREAT | O_EXCL | O_WRONLY, 0666);\n-\n-\tif (report < 0)\n-\t\tdie(_(\"couldn't create a new file at '%s'\"), report_path.buf);\n+\treport = xopen(report_path.buf, O_CREAT | O_EXCL | O_WRONLY, 0666);\n\n \tif (write_in_full(report, buffer.buf, buffer.len) < 0)\n \t\tdie_errno(_(\"unable to write to %s\"), report_path.buf);\ndiff --git a/builtin/commit-tree.c b/builtin/commit-tree.c\nindex 1031b9a491..63ea322933 100644\n--- a/builtin/commit-tree.c\n+++ b/builtin/commit-tree.c\n@@ -88,9 +88,7 @@ static int parse_file_arg_callback(const struct option *opt,\n \tif (!strcmp(arg, \"-\"))\n \t\tfd = 0;\n \telse {\n-\t\tfd = open(arg, O_RDONLY);\n-\t\tif (fd < 0)\n-\t\t\tdie_errno(_(\"git commit-tree: failed to open '%s'\"), arg);\n+\t\tfd = xopen(arg, O_RDONLY);\n \t}\n \tif (strbuf_read(buf, fd, 0) < 0)\n \t\tdie_errno(_(\"git commit-tree: failed to read '%s'\"), arg);\ndiff --git a/builtin/hash-object.c b/builtin/hash-object.c\nindex 640ef4ded5..2e6e2ddd0c 100644\n--- a/builtin/hash-object.c\n+++ b/builtin/hash-object.c\n@@ -53,9 +53,7 @@ static void hash_object(const char *path, const char *type, const char *vpath,\n \t\t\tunsigned flags, int literally)\n {\n \tint fd;\n-\tfd = open(path, O_RDONLY);\n-\tif (fd < 0)\n-\t\tdie_errno(\"Cannot open '%s'\", path);\n+\tfd = xopen(path, O_RDONLY);\n \thash_fd(fd, type, vpath, flags, literally);\n }\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 8336466865..6cc4890217 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -338,15 +338,11 @@ static const char *open_pack_file(const char *pack_name)\n \t\t\t\t\t\t\"pack/tmp_pack_XXXXXX\");\n \t\t\tpack_name = strbuf_detach(&tmp_file, NULL);\n \t\t} else {\n-\t\t\toutput_fd = open(pack_name, O_CREAT|O_EXCL|O_RDWR, 0600);\n-\t\t\tif (output_fd < 0)\n-\t\t\t\tdie_errno(_(\"unable to create '%s'\"), pack_name);\n+\t\t\toutput_fd = xopen(pack_name, O_CREAT|O_EXCL|O_RDWR, 0600);\n \t\t}\n \t\tnothread_data.pack_fd = output_fd;\n \t} else {\n-\t\tinput_fd = open(pack_name, O_RDONLY);\n-\t\tif (input_fd < 0)\n-\t\t\tdie_errno(_(\"cannot open packfile '%s'\"), pack_name);\n+\t\tinput_fd = xopen(pack_name, O_RDONLY);\n \t\toutput_fd = -1;\n \t\tnothread_data.pack_fd = input_fd;\n \t}\ndiff --git a/builtin/mailsplit.c b/builtin/mailsplit.c\nindex 664400b816..7baef30569 100644\n--- a/builtin/mailsplit.c\n+++ b/builtin/mailsplit.c\n@@ -75,9 +75,7 @@ static int split_one(FILE *mbox, const char *name, int allow_bare)\n \t\tfprintf(stderr, \"corrupt mailbox\\n\");\n \t\texit(1);\n \t}\n-\tfd = open(name, O_WRONLY | O_CREAT | O_EXCL, 0666);\n-\tif (fd < 0)\n-\t\tdie_errno(\"cannot open output file '%s'\", name);\n+\tfd = xopen(name, O_WRONLY | O_CREAT | O_EXCL, 0666);\n \toutput = xfdopen(fd, \"w\");\n\n \t/* Copy it out, while searching for a line that begins with\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 22f23990b3..47614d8070 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -1136,9 +1136,7 @@ static void handle_fetch_head(struct commit_list **remotes, struct strbuf *merge\n \t\tmerge_names = &fetch_head_file;\n\n \tfilename = git_path_fetch_head(the_repository);\n-\tfd = open(filename, O_RDONLY);\n-\tif (fd < 0)\n-\t\tdie_errno(_(\"could not open '%s' for reading\"), filename);\n+\tfd = xopen(filename, O_RDONLY);\n\n \tif (strbuf_read(merge_names, fd, 0) < 0)\n \t\tdie_errno(_(\"could not read '%s'\"), filename);\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 74bba39ca8..71c59583a1 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -172,9 +172,7 @@ static void prepare_note_data(const struct object_id *object, struct note_data *\n\n \t\t/* write the template message before editing: */\n \t\td->edit_path = git_pathdup(\"NOTES_EDITMSG\");\n-\t\tfd = open(d->edit_path, O_CREAT | O_TRUNC | O_WRONLY, 0600);\n-\t\tif (fd < 0)\n-\t\t\tdie_errno(_(\"could not create file '%s'\"), d->edit_path);\n+\t\tfd = xopen(d->edit_path, O_CREAT | O_TRUNC | O_WRONLY, 0600);\n\n \t\tif (d->given)\n \t\t\twrite_or_die(fd, d->buf.buf, d->buf.len);\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 452558ec95..065b6bf093 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -293,9 +293,7 @@ static void create_tag(const struct object_id *object, const char *object_ref,\n\n \t\t/* write the template message before editing: */\n \t\tpath = git_pathdup(\"TAG_EDITMSG\");\n-\t\tfd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0600);\n-\t\tif (fd < 0)\n-\t\t\tdie_errno(_(\"could not create file '%s'\"), path);\n+\t\tfd = xopen(path, O_CREAT | O_TRUNC | O_WRONLY, 0600);\n\n \t\tif (opt->message_given) {\n \t\t\twrite_or_die(fd, buf->buf, buf->len);\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex f1f16f2de5..187203e8bb 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -95,9 +95,7 @@ static int create_file(const char *path)\n {\n \tint fd;\n \tpath = get_mtime_path(path);\n-\tfd = open(path, O_CREAT | O_RDWR, 0644);\n-\tif (fd < 0)\n-\t\tdie_errno(_(\"failed to create file %s\"), path);\n+\tfd = xopen(path, O_CREAT | O_RDWR, 0644);\n \treturn fd;\n }\n\ndiff --git a/contrib/coccinelle/xopen.cocci b/contrib/coccinelle/xopen.cocci\nnew file mode 100644\nindex 0000000000..814d7b8a1a\n--- /dev/null\n+++ b/contrib/coccinelle/xopen.cocci\n@@ -0,0 +1,16 @@\n+@@\n+identifier fd;\n+identifier die_fn =~ \"^(die|die_errno)$\";\n+@@\n+(\n+  fd =\n+- open\n++ xopen\n+  (...);\n+|\n+  int fd =\n+- open\n++ xopen\n+  (...);\n+)\n+- if ( \\( fd < 0 \\| fd == -1 \\) ) { die_fn(...); }\ndiff --git a/csum-file.c b/csum-file.c\nindex c951cf8277..26e8a6df44 100644\n--- a/csum-file.c\n+++ b/csum-file.c\n@@ -131,12 +131,8 @@ struct hashfile *hashfd_check(const char *name)\n \tint sink, check;\n \tstruct hashfile *f;\n\n-\tsink = open(\"/dev/null\", O_WRONLY);\n-\tif (sink < 0)\n-\t\tdie_errno(\"unable to open /dev/null\");\n-\tcheck = open(name, O_RDONLY);\n-\tif (check < 0)\n-\t\tdie_errno(\"unable to open '%s'\", name);\n+\tsink = xopen(\"/dev/null\", O_WRONLY);\n+\tcheck = xopen(name, O_RDONLY);\n \tf = hashfd(sink, name);\n \tf->check_fd = check;\n \tf->check_buffer = xmalloc(f->buffer_len);\ndiff --git a/pack-write.c b/pack-write.c\nindex f1fc3ecafa..2767b78619 100644\n--- a/pack-write.c\n+++ b/pack-write.c\n@@ -75,9 +75,7 @@ const char *write_idx_file(const char *index_name, struct pack_idx_entry **objec\n \t\t\tindex_name = strbuf_detach(&tmp_file, NULL);\n \t\t} else {\n \t\t\tunlink(index_name);\n-\t\t\tfd = open(index_name, O_CREAT|O_EXCL|O_WRONLY, 0600);\n-\t\t\tif (fd < 0)\n-\t\t\t\tdie_errno(\"unable to create '%s'\", index_name);\n+\t\t\tfd = xopen(index_name, O_CREAT|O_EXCL|O_WRONLY, 0600);\n \t\t}\n \t\tf = hashfd(fd, index_name);\n \t}\n@@ -256,9 +254,7 @@ const char *write_rev_file_order(const char *rev_name,\n \t\t\trev_name = strbuf_detach(&tmp_file, NULL);\n \t\t} else {\n \t\t\tunlink(rev_name);\n-\t\t\tfd = open(rev_name, O_CREAT|O_EXCL|O_WRONLY, 0600);\n-\t\t\tif (fd < 0)\n-\t\t\t\tdie_errno(\"unable to create '%s'\", rev_name);\n+\t\t\tfd = xopen(rev_name, O_CREAT|O_EXCL|O_WRONLY, 0600);\n \t\t}\n \t\tf = hashfd(fd, rev_name);\n \t} else if (flags & WRITE_REV_VERIFY) {\ndiff --git a/run-command.c b/run-command.c\nindex f72e72cce7..2961f7e55e 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -761,9 +761,7 @@ int start_command(struct child_process *cmd)\n \t\tnotify_pipe[0] = notify_pipe[1] = -1;\n\n \tif (cmd->no_stdin || cmd->no_stdout || cmd->no_stderr) {\n-\t\tnull_fd = open(\"/dev/null\", O_RDWR | O_CLOEXEC);\n-\t\tif (null_fd < 0)\n-\t\t\tdie_errno(_(\"open /dev/null failed\"));\n+\t\tnull_fd = xopen(\"/dev/null\", O_RDWR | O_CLOEXEC);\n \t\tset_cloexec(null_fd);\n \t}\n\n--\n2.33.0\n"},{"id":"433788","messageId":"CAPUEspjkcV1_R5DNXCkL5wQpZCW+K4As2nGuEGu6fyeFrr15KQ@mail.gmail.com","threadId":"56377","inReplyTo":"6a5c3e8e-0216-8b63-38fa-b7b19331d752@web.de","subject":"Re: [PATCH 1/2] xopen: explicitly report creation failures","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2021-08-25T23:46:54Z","receivedAt":"2021-08-25T23:47:34Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Wed, Aug 25, 2021 at 2:11 PM René Scharfe <l.s.r@web.de> wrote:\n>\n> diff --git a/wrapper.c b/wrapper.c\n> index 563ad590df..7c6586af32 100644\n> --- a/wrapper.c\n> +++ b/wrapper.c\n> @@ -193,7 +193,9 @@ int xopen(const char *path, int oflag, ...)\n>                 if (errno == EINTR)\n>                         continue;\n>\n> -               if ((oflag & O_RDWR) == O_RDWR)\n> +               if ((oflag & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL))\n> +                       die_errno(_(\"unable to create '%s'\"), path);\n\nprobably over conservative, but && errno == EEXIST?\n\n> +               else if ((oflag & O_RDWR) == O_RDWR)\n>                         die_errno(_(\"could not open '%s' for reading and writing\"), path);\n>                 else if ((oflag & O_WRONLY) == O_WRONLY)\n>                         die_errno(_(\"could not open '%s' for writing\"), path);\n\nSince you are already changing this code, why not take the opportunity\nto refactor it\nand remove the \" == FLAG\" part of these conditionals which is\notherwise redundant?\n\nEither way \"Reviewed-by\", and indeed a nice cleanup.\n\nCarlo\n"},{"id":"433840","messageId":"f44bf316-efae-34ce-33e0-0161c3bb78a0@web.de","threadId":"56377","inReplyTo":"CAPUEspjkcV1_R5DNXCkL5wQpZCW+K4As2nGuEGu6fyeFrr15KQ@mail.gmail.com","subject":"Re: [PATCH 1/2] xopen: explicitly report creation failures","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2021-08-26T15:23:27Z","receivedAt":"2021-08-26T15:23:35Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 26.08.21 um 01:46 schrieb Carlo Arenas:\n> On Wed, Aug 25, 2021 at 2:11 PM René Scharfe <l.s.r@web.de> wrote:\n>>\n>> diff --git a/wrapper.c b/wrapper.c\n>> index 563ad590df..7c6586af32 100644\n>> --- a/wrapper.c\n>> +++ b/wrapper.c\n>> @@ -193,7 +193,9 @@ int xopen(const char *path, int oflag, ...)\n>>                 if (errno == EINTR)\n>>                         continue;\n>>\n>> -               if ((oflag & O_RDWR) == O_RDWR)\n>> +               if ((oflag & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL))\n>> +                       die_errno(_(\"unable to create '%s'\"), path);\n>\n> probably over conservative, but && errno == EEXIST?\n\nNo matter what error we got, if O_CREAT and O_EXCL were both given then\nwe tried to create a file, so this message applies.\n\n>\n>> +               else if ((oflag & O_RDWR) == O_RDWR)\n>>                         die_errno(_(\"could not open '%s' for reading and writing\"), path);\n>>                 else if ((oflag & O_WRONLY) == O_WRONLY)\n>>                         die_errno(_(\"could not open '%s' for writing\"), path);\n>\n> Since you are already changing this code, why not take the opportunity\n> to refactor it\n> and remove the \" == FLAG\" part of these conditionals which is\n> otherwise redundant?\n\nThe repetition is unsightly, but it's a different issue that should be\naddressed separately.  Simply removing the comparison feels iffy,\nthough.  POSIX doesn't seem to forbid e.g. O_RDONLY to be 1, O_WRONLY\nto be 2 and O_RDWR to be 3, and then you need to check all masked bits.\nI can't think of simpler alternative to the comparison.\n\n> Either way \"Reviewed-by\", and indeed a nice cleanup.\n\nThank you!\n\nRené\n"},{"id":"433849","messageId":"xmqqa6l4f8x4.fsf@gitster.g","threadId":"56377","inReplyTo":"f44bf316-efae-34ce-33e0-0161c3bb78a0@web.de","subject":"Re: [PATCH 1/2] xopen: explicitly report creation failures","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-26T16:49:27Z","receivedAt":"2021-08-26T16:49:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> Am 26.08.21 um 01:46 schrieb Carlo Arenas:\n>> On Wed, Aug 25, 2021 at 2:11 PM René Scharfe <l.s.r@web.de> wrote:\n>>>\n>>> diff --git a/wrapper.c b/wrapper.c\n>>> index 563ad590df..7c6586af32 100644\n>>> --- a/wrapper.c\n>>> +++ b/wrapper.c\n>>> @@ -193,7 +193,9 @@ int xopen(const char *path, int oflag, ...)\n>>>                 if (errno == EINTR)\n>>>                         continue;\n>>>\n>>> -               if ((oflag & O_RDWR) == O_RDWR)\n>>> +               if ((oflag & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL))\n>>> +                       die_errno(_(\"unable to create '%s'\"), path);\n>>\n>> probably over conservative, but && errno == EEXIST?\n>\n> No matter what error we got, if O_CREAT and O_EXCL were both given then\n> we tried to create a file, so this message applies.\n\n100% agreed.\n\n>>> +               else if ((oflag & O_RDWR) == O_RDWR)\n>>>                         die_errno(_(\"could not open '%s' for reading and writing\"), path);\n>>>                 else if ((oflag & O_WRONLY) == O_WRONLY)\n>>>                         die_errno(_(\"could not open '%s' for writing\"), path);\n>>\n>> Since you are already changing this code, why not take the opportunity\n>> to refactor it\n>> and remove the \" == FLAG\" part of these conditionals which is\n>> otherwise redundant?\n>\n> The repetition is unsightly, but it's a different issue that should be\n> addressed separately.  Simply removing the comparison feels iffy,\n> though.  POSIX doesn't seem to forbid e.g. O_RDONLY to be 1, O_WRONLY\n> to be 2 and O_RDWR to be 3, and then you need to check all masked bits.\n> I can't think of simpler alternative to the comparison.\n\nI fully agree that such a change, if done, must be done in an\nunrelated patch.  \n\nIt is funny that the code is already prepared for such a case where\nRDWR is defined as RDONLY|WRONLY.  I wonder if we wrote the series\nof comparisons in this order on purpose, or we were just lucky, when\nwe did 3ff53df7 (wrapper: implement xopen(), 2015-08-04) ;-)\n\n\n>\n>> Either way \"Reviewed-by\", and indeed a nice cleanup.\n>\n> Thank you!\n\nYes, indeed, this is nicely done.\n"}]}