threads / patch / 56475

patchrefs/files-backend: remove unused open mode parameter

Subject: [PATCH] refs/files-backend: remove unused open mode parameter

## tl;dr

5 messages between Sep 9, 2021 and Sep 13, 2021. Diffs are folded; open one to read it.

replies: 4people: 4as markdown or json

René Scharfe· Sep 9, 2021, 21:45 UTC · lore

We only need to provide a mode if we are willing to let open(2) create the file, which is not the case here, so drop the unnecessary parameter.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 refs/files-backend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to refs/files-backend.c +1 −2
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 677b7e4cdd..74c0385873 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1569,7 +1569,7 @@ static int log_ref_setup(struct files_ref_store *refs,
 			goto error;
 		}
 	} else {
-		*logfd = open(logfile, O_APPEND | O_WRONLY, 0666);
+		*logfd = open(logfile, O_APPEND | O_WRONLY);
 		if (*logfd < 0) {
 			if (errno == ENOENT || errno == EISDIR) {
 				/*
--
2.33.0
Junio C Hamano· Sep 10, 2021, 00:45 UTC · re: René Scharfe · lore

Re: [PATCH] refs/files-backend: remove unused open mode parameter

René Scharfe <l.s.r@web.de> writes:
> We only need to provide a mode if we are willing to let open(2) create
> the file, which is not the case here, so drop the unnecessary parameter.
Obviously correct.
$ git grep -e 'open.*0[0-7][0-7][0-7]' seen \*.c | grep -v CREAT
gives this one and another from reftable.
-- >8 --
Subject: [PATCH] fixup! reftable: implement stack, a mutable database of reftable files.
To be squashed into the said commit.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 reftable/stack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to reftable/stack.c +1 −1
diff --git a/reftable/stack.c b/reftable/stack.c
index 48e22a6c18..df5021ebf0 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -107,7 +107,7 @@ static int fd_read_lines(int fd, char ***namesp)
 
 int read_lines(const char *filename, char ***namesp)
 {
-	int fd = open(filename, O_RDONLY, 0644);
+	int fd = open(filename, O_RDONLY);
 	int err = 0;
 	if (fd < 0) {
 		if (errno == ENOENT) {

-- 
2.33.0-484-g4fa4683b52
Han-Wen Nienhuys· Sep 13, 2021, 10:12 UTC · re: René Scharfe · lore

Re: [PATCH] refs/files-backend: remove unused open mode parameter

On Thu, Sep 9, 2021 at 11:46 PM René Scharfe <l.s.r@web.de> wrote:
>
> We only need to provide a mode if we are willing to let open(2) create
> the file, which is not the case here, so drop the unnecessary parameter.

I was #today years old when I learned that C supports (a limited form of) function signature overloading.

LGTM
-- 
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--
Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Junio C Hamano· Sep 13, 2021, 18:22 UTC · re: Han-Wen Nienhuys · lore

Re: [PATCH] refs/files-backend: remove unused open mode parameter

Han-Wen Nienhuys <hanwen@google.com> writes:
Show 7 quoted lines
> On Thu, Sep 9, 2021 at 11:46 PM René Scharfe <l.s.r@web.de> wrote:
>>
>> We only need to provide a mode if we are willing to let open(2) create
>> the file, which is not the case here, so drop the unnecessary parameter.
>
> I was #today years old when I learned that C supports (a limited form
> of) function signature overloading.
I do not think it is that kind of magic.

Like printf(3) that allows its early parameter to affect the way how its later parameters are recognised, it just allows the flags word to decide if it needs to grab one extra mode_t out of va_list or not, which can be done as a plain vanilla varargs function, i.e.

	extern int open(const char *path, int flags, ...);
Jeff King· Sep 13, 2021, 19:03 UTC · re: Junio C Hamano · lore

Re: [PATCH] refs/files-backend: remove unused open mode parameter

On Mon, Sep 13, 2021 at 11:22:49AM -0700, Junio C Hamano wrote:
Show 18 quoted lines
> Han-Wen Nienhuys <hanwen@google.com> writes:
> 
> > On Thu, Sep 9, 2021 at 11:46 PM René Scharfe <l.s.r@web.de> wrote:
> >>
> >> We only need to provide a mode if we are willing to let open(2) create
> >> the file, which is not the case here, so drop the unnecessary parameter.
> >
> > I was #today years old when I learned that C supports (a limited form
> > of) function signature overloading.
> 
> I do not think it is that kind of magic.
> 
> Like printf(3) that allows its early parameter to affect the way how
> its later parameters are recognised, it just allows the flags word
> to decide if it needs to grab one extra mode_t out of va_list or
> not, which can be done as a plain vanilla varargs function, i.e.
> 
> 	extern int open(const char *path, int flags, ...);

Yes, you can see some examples (complete with interesting subtleties) in wrapper.c:xopen() and compat/open.c:git_open_with_retry(). :)

-Peff

← back to recent threads