Re: [PATCH 2/2] config: use chmod() instead of fchmod()

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 2/2] config: use chmod() instead of fchmod()

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:01:57

Karsten Blees [off-list ref] writes:
There is no fchmod() on native Windows platforms (MinGW and MSVC), and the
equivalent Win32 API (SetFileInformationByHandle) requires Windows Vista.

Use chmod() instead.

Signed-off-by: Karsten Blees <redacted>
---
I am wondering if it is saner to just revert the fchmod() patch and
replace it with something along the lines of

http://thread.gmane.org/gmane.comp.version-control.git/251682/focus=253219

Having said that, these are the only two callers of fchmod()
currently in our code base, so I'll queue this patch to allow us to
kick the problem-can down the road ;-)

Thanks.
quoted hunk
 config.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/config.c b/config.c
index ba882a1..9767c4b 100644
--- a/config.c
+++ b/config.c
@@ -1636,8 +1636,8 @@ int git_config_set_multivar_in_file(const char *config_filename,
 			MAP_PRIVATE, in_fd, 0);
 		close(in_fd);
 
-		if (fchmod(fd, st.st_mode & 07777) < 0) {
-			error("fchmod on %s failed: %s",
+		if (chmod(lock->filename, st.st_mode & 07777) < 0) {
+			error("chmod on %s failed: %s",
 				lock->filename, strerror(errno));
 			ret = CONFIG_NO_WRITE;
 			goto out_free;
@@ -1815,8 +1815,8 @@ int git_config_rename_section_in_file(const char *config_filename,
 
 	fstat(fileno(config_file), &st);
 
-	if (fchmod(out_fd, st.st_mode & 07777) < 0) {
-		ret = error("fchmod on %s failed: %s",
+	if (chmod(lock->filename, st.st_mode & 07777) < 0) {
+		ret = error("chmod on %s failed: %s",
 				lock->filename, strerror(errno));
 		goto out;
 	}
-- 
2.0.1.779.g26aeac4.dirty

-- 
-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 2/2] config: use chmod() instead of fchmod()

From: Karsten Blees <hidden>
Date: 2016-06-15 23:01:57

Am 17.07.2014 00:16, schrieb Junio C Hamano:
Karsten Blees [off-list ref] writes:
quoted
There is no fchmod() on native Windows platforms (MinGW and MSVC), and the
equivalent Win32 API (SetFileInformationByHandle) requires Windows Vista.

Use chmod() instead.

Signed-off-by: Karsten Blees <redacted>
---
I am wondering if it is saner to just revert the fchmod() patch and
replace it with something along the lines of

http://thread.gmane.org/gmane.comp.version-control.git/251682/focus=253219
I also think it makes a lot of sense to handle permissions centrally.

However, with this patch, the permissions of the target file will
additionally be limited by umask (by passing them to open()), and then
overridden completely if core.sharedRepository is set.

Perhaps the lockfile API should respect the location of the lock files
(i.e. use core.sharedRepository in .git, 0666 in the work-tree, and
copy permissions anywhere else).

Another thing I find strange is that, by doing copy/replace, git silently
overwrites readonly files. If we grab the permissions from the source
file anyway, we should perhaps add 'if (!(perms & 0222)) error("file
is readonly");', or even 'access(filename, W_OK)'?
Having said that, these are the only two callers of fchmod()
currently in our code base, so I'll queue this patch to allow us to
kick the problem-can down the road ;-)
Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help