Hi,
Jeff King wrote:
> The store_write_section() and store_write_pairs() functions
> are basically high-level wrappers around write(). But their
> return values are flipped from our usual convention, using
> "1" for success and "0" for failure.
>
> Let's flip them to follow the usual write() conventions and
> update all callers. As these are local to config.c, it's
> unlikely that we'd have new callers in any topics in flight
> (which would be silently broken by our change). But just to
> be on the safe side, let's rename them to just
> write_section() and write_pairs(). That also accentuates
> their relationship with write().
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
The caller only cares if it succeeded, right? Could this return
the customary 0 vs -1 instead of the number of bytes written?
That would look like the following (as a patch to squash in):
With or without that tweak,
Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
diff --git i/config.c w/config.c
index 272a32aac0..8f92d452bf 100644
--- i/config.c
+++ w/config.c
@@ -2297,11 +2297,10 @@ static int write_error(const char *filename)
return 4;
}
-static ssize_t write_section(int fd, const char *key)
+static int write_section(int fd, const char *key)
{
const char *dot;
- int i;
- ssize_t ret;
+ int i, ret;
struct strbuf sb = STRBUF_INIT;
dot = memchr(key, '.', store.baselen);
@@ -2317,16 +2316,15 @@ static ssize_t write_section(int fd, const char *key)
strbuf_addf(&sb, "[%.*s]\n", store.baselen, key);
}
- ret = write_in_full(fd, sb.buf, sb.len);
+ ret = write_in_full(fd, sb.buf, sb.len) < 0 ? -1 : 0;
strbuf_release(&sb);
return ret;
}
-static ssize_t write_pair(int fd, const char *key, const char *value)
+static int write_pair(int fd, const char *key, const char *value)
{
- int i;
- ssize_t ret;
+ int i, ret;
int length = strlen(key + store.baselen + 1);
const char *quote = "";
struct strbuf sb = STRBUF_INIT;
@@ -2366,7 +2364,7 @@ static ssize_t write_pair(int fd, const char *key, const char *value)
}
strbuf_addf(&sb, "%s\n", quote);
- ret = write_in_full(fd, sb.buf, sb.len);
+ ret = write_in_full(fd, sb.buf, sb.len) < 0 ? -1 : 0;
strbuf_release(&sb);
return ret;
@@ -2499,8 +2497,7 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
}
store.key = (char *)key;
- if (write_section(fd, key) < 0 ||
- write_pair(fd, key, value) < 0)
+ if (write_section(fd, key) || write_pair(fd, key, value))
goto write_err_out;
} else {
struct stat st;
@@ -2623,10 +2620,10 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
/* write the pair (value == NULL means unset) */
if (value != NULL) {
if (store.state == START) {
- if (write_section(fd, key) < 0)
+ if (write_section(fd, key))
goto write_err_out;
}
- if (write_pair(fd, key, value) < 0)
+ if (write_pair(fd, key, value))
goto write_err_out;
}
@@ -2820,7 +2817,7 @@ int git_config_rename_section_in_file(const char *config_filename,
continue;
}
store.baselen = strlen(new_name);
- if (write_section(out_fd, new_name) < 0) {
+ if (write_section(out_fd, new_name)) {
ret = write_error(get_lock_file_path(lock));
goto out;
}
