diff mbox series

refs/files-backend: remove unused open mode parameter

Message ID ea424e3d-6269-f50d-1a4a-643bb95cfa12@web.de (mailing list archive)
State Accepted
Commit a3ea25c5f19427407086c794f7e28b975813a8fe
Headers show
Series refs/files-backend: remove unused open mode parameter | expand

Commit Message

René Scharfe Sept. 9, 2021, 9:45 p.m. UTC
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(-)

--
2.33.0

Comments

Junio C Hamano Sept. 10, 2021, 12:45 a.m. UTC | #1
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) {
Han-Wen Nienhuys Sept. 13, 2021, 10:12 a.m. UTC | #2
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
Junio C Hamano Sept. 13, 2021, 6:22 p.m. UTC | #3
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, ...);
Jeff King Sept. 13, 2021, 7:03 p.m. UTC | #4
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
diff mbox series

Patch

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) {
 				/*