# use result of open(2) to check for presence 5 messages from 2006-01-05 to 2006-01-05. Participants: Alex Riesen, Morten Welinder. Thread: https://gitlist.dev/t/2972 ## Alex Riesen, 2006-01-05 11:43 Subject: use result of open(2) to check for presence Message-ID: <81b0412b0601050343w4c00ac86y30569c3babbf6728@mail.gmail.com> URL: https://gitlist.dev/e/81b0412b0601050343w4c00ac86y30569c3babbf6728%40mail.gmail.com ``` Not that the stat against open race would matter much in this context, but that simplifies the code a bit. Also some diagnostics added (why the open failed) Signed-off-by: Alex Riesen Subject: [PATCH] use result of open(2) to check for presence of the old config file Signed-off-by: Alex Riesen --- config.c | 17 ++++++++++++----- 1 files changed, 12 insertions(+), 5 deletions(-) b74ea5a71504808112bd4781a69b17f6009104db diff --git a/config.c b/config.c index 992e988..8355224 100644 --- a/config.c +++ b/config.c @@ -409,8 +409,7 @@ int git_config_set_multivar(const char* const char* value_regex, int multi_replace) { int i; - struct stat st; - int fd; + int fd, in_fd; char* config_filename = strdup(git_path("config")); char* lock_file = strdup(git_path("config.lock")); const char* last_dot = strrchr(key, '.'); @@ -457,9 +456,17 @@ int git_config_set_multivar(const char* /* * If .git/config does not exist yet, write a minimal version. */ - if (stat(config_filename, &st)) { + in_fd = open(config_filename, O_RDONLY); + if ( in_fd < 0 ) { free(store.key); + if ( ENOENT != errno ) { + error("opening %s: %s", config_filename, + strerror(errno)); + close(fd); + unlink(lock_file); + return 3; /* same as "invalid config file" */ + } /* if nothing to unset, error out */ if (value == NULL) { close(fd); @@ -471,7 +478,7 @@ int git_config_set_multivar(const char* store_write_section(fd, key); store_write_pair(fd, key, value); } else{ - int in_fd; + struct stat st; char* contents; int i, copy_begin, copy_end, new_line = 0; @@ -528,7 +535,7 @@ int git_config_set_multivar(const char* return 5; } - in_fd = open(config_filename, O_RDONLY, 0666); + fstat(in_fd, &st); contents = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, in_fd, 0); close(in_fd); -- 1.0.GIT ``` ## Morten Welinder, 2006-01-05 14:48 Subject: Re: use result of open(2) to check for presence Message-ID: <118833cc0601050648t17f68bcch8fd7c541585e965c@mail.gmail.com> URL: https://gitlist.dev/e/118833cc0601050648t17f68bcch8fd7c541585e965c%40mail.gmail.com In-Reply-To: <81b0412b0601050343w4c00ac86y30569c3babbf6728@mail.gmail.com> ``` Neat, but perhaps you should check the fstat return code. It actually can fail, for example if the filesize cannot be represented in the structure because it is too big. Morten ``` ## Alex Riesen, 2006-01-05 15:12 Subject: Re: use result of open(2) to check for presence Message-ID: <81b0412b0601050712q4c2123f7i8e9190c3b75b6fda@mail.gmail.com> URL: https://gitlist.dev/e/81b0412b0601050712q4c2123f7i8e9190c3b75b6fda%40mail.gmail.com In-Reply-To: <118833cc0601050648t17f68bcch8fd7c541585e965c@mail.gmail.com> ``` On 1/5/06, Morten Welinder wrote: > Neat, but perhaps you should check the fstat return code. > It actually can fail, for example if the filesize cannot be represented in the > structure because it is too big. Yes, of course. It will be added along with the missing test for mmap returning MAP_FAILED. Sometime. Sarcasm aside, how do you get such a situation? ``` ## Morten Welinder, 2006-01-05 16:45 Subject: Re: use result of open(2) to check for presence Message-ID: <118833cc0601050845i40f93bbajdd70e77953d07014@mail.gmail.com> URL: https://gitlist.dev/e/118833cc0601050845i40f93bbajdd70e77953d07014%40mail.gmail.com In-Reply-To: <81b0412b0601050712q4c2123f7i8e9190c3b75b6fda@mail.gmail.com> ``` Config files always tend to grow, but I do agree that 4GB config files stretches credibility a bit. However, empty ones are not unlikely and then mmap will fail. M. ``` ## Alex Riesen, 2006-01-05 18:07 Subject: Re: use result of open(2) to check for presence Message-ID: <20060105180715.GA6112@steel.home> URL: https://gitlist.dev/e/20060105180715.GA6112%40steel.home In-Reply-To: <118833cc0601050845i40f93bbajdd70e77953d07014@mail.gmail.com> ``` Morten Welinder, Thu, Jan 05, 2006 17:45:27 +0100: > Config files always tend to grow, but I do agree that 4GB config files stretches > credibility a bit. > > However, empty ones are not unlikely and then mmap will fail. > Shouldn't be a problem: the code in question does not seem to do anything for empty files and wont even touch the result of mmap. ```