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

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

[René Scharfe]

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(-)

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

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

[Junio C Hamano]

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(-)

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

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

[Han-Wen Nienhuys]

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

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

[Junio C Hamano]

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, ...);

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

[Jeff King]

On Mon, Sep 13, 2021 at 11:22:49AM -0700, Junio C Hamano wrote:

> 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