Code

use write_str_in_full helper to avoid literal string lengths
authorJim Meyering <jim@meyering.net>
Sat, 12 Sep 2009 08:54:32 +0000 (10:54 +0200)
committerJunio C Hamano <gitster@pobox.com>
Sun, 13 Sep 2009 08:31:10 +0000 (01:31 -0700)
In 2d14d65 (Use a clearer style to issue commands to remote helpers,
2009-09-03) I happened to notice two changes like this:

- write_in_full(helper->in, "list\n", 5);
+
+ strbuf_addstr(&buf, "list\n");
+ write_in_full(helper->in, buf.buf, buf.len);
+ strbuf_reset(&buf);

IMHO, it would be better to define a new function,

    static inline ssize_t write_str_in_full(int fd, const char *str)
    {
           return write_in_full(fd, str, strlen(str));
    }

and then use it like this:

-       strbuf_addstr(&buf, "list\n");
-       write_in_full(helper->in, buf.buf, buf.len);
-       strbuf_reset(&buf);
+       write_str_in_full(helper->in, "list\n");

Thus not requiring the added allocation, and still avoiding
the maintenance risk of literal string lengths.
These days, compilers are good enough that strlen("literal")
imposes no run-time cost.

Transformed via this:

    perl -pi -e \
        's/write_in_full\((.*?), (".*?"), \d+\)/write_str_in_full($1, $2)/'\
      $(git grep -l 'write_in_full.*"')

Signed-off-by: Jim Meyering <meyering@redhat.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin-fetch.c
builtin-reflog.c
cache.h
commit.c
config.c
rerere.c
upload-pack.c

index 817dd6bff0bf0ddc9a3d00191bb3c422300b878b..cb48c57ca3e66b7ec39a98128b2cfb058c2dad15 100644 (file)
@@ -454,7 +454,7 @@ static int quickfetch(struct ref *ref_map)
 
        for (ref = ref_map; ref; ref = ref->next) {
                if (write_in_full(revlist.in, sha1_to_hex(ref->old_sha1), 40) < 0 ||
-                   write_in_full(revlist.in, "\n", 1) < 0) {
+                   write_str_in_full(revlist.in, "\n") < 0) {
                        if (errno != EPIPE && errno != EINVAL)
                                error("failed write to rev-list: %s", strerror(errno));
                        err = -1;
index 95198c5de41072bfb8adf7f29d9bbb44eec665ac..e23b5ef979d98e5e44693345856a13485695246f 100644 (file)
@@ -362,7 +362,7 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,
                } else if (cmd->updateref &&
                        (write_in_full(lock->lock_fd,
                                sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
-                        write_in_full(lock->lock_fd, "\n", 1) != 1 ||
+                        write_str_in_full(lock->lock_fd, "\n") != 1 ||
                         close_ref(lock) < 0)) {
                        status |= error("Couldn't write %s",
                                lock->lk->filename);
diff --git a/cache.h b/cache.h
index 5fad24ce219be746ad582d8432464753975f34fe..867918d0417642b7c1bb6ccb8f88730fdea08157 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -923,13 +923,18 @@ extern const char *git_mailmap_file;
 extern void maybe_flush_or_die(FILE *, const char *);
 extern int copy_fd(int ifd, int ofd);
 extern int copy_file(const char *dst, const char *src, int mode);
-extern ssize_t read_in_full(int fd, void *buf, size_t count);
-extern ssize_t write_in_full(int fd, const void *buf, size_t count);
 extern void write_or_die(int fd, const void *buf, size_t count);
 extern int write_or_whine(int fd, const void *buf, size_t count, const char *msg);
 extern int write_or_whine_pipe(int fd, const void *buf, size_t count, const char *msg);
 extern void fsync_or_die(int fd, const char *);
 
+extern ssize_t read_in_full(int fd, void *buf, size_t count);
+extern ssize_t write_in_full(int fd, const void *buf, size_t count);
+static inline ssize_t write_str_in_full(int fd, const char *str)
+{
+       return write_in_full(fd, str, strlen(str));
+}
+
 /* pager.c */
 extern void setup_pager(void);
 extern const char *pager_program;
index a6c6f70a9237385d163ea12ef28e5087e9f80411..fedbd5e5267ec4beaf1f245a879918a2685c10b9 100644 (file)
--- a/commit.c
+++ b/commit.c
@@ -212,7 +212,7 @@ int write_shallow_commits(int fd, int use_pack_protocol)
                        else {
                                if (write_in_full(fd, hex,  40) != 40)
                                        break;
-                               if (write_in_full(fd, "\n", 1) != 1)
+                               if (write_str_in_full(fd, "\n") != 1)
                                        break;
                        }
                }
index e87edeab0c6b9579ecbd9bc5a9a11c3522d21ccf..bf122202a014160321d97444a9b3bf2b546f6f3f 100644 (file)
--- a/config.c
+++ b/config.c
@@ -1116,7 +1116,7 @@ int git_config_set_multivar(const char *key, const char *value,
                                    copy_end - copy_begin)
                                        goto write_err_out;
                                if (new_line &&
-                                   write_in_full(fd, "\n", 1) != 1)
+                                   write_str_in_full(fd, "\n") != 1)
                                        goto write_err_out;
                        }
                        copy_begin = store.offset[i];
index 87360dc23e54aabf47b3bda160e58058507d4a2f..29f95f657d04300e375d007e9d01832b89311602 100644 (file)
--- a/rerere.c
+++ b/rerere.c
@@ -61,7 +61,7 @@ static int write_rr(struct string_list *rr, int out_fd)
                path = rr->items[i].string;
                length = strlen(path) + 1;
                if (write_in_full(out_fd, rr->items[i].util, 40) != 40 ||
-                   write_in_full(out_fd, "\t", 1) != 1 ||
+                   write_str_in_full(out_fd, "\t") != 1 ||
                    write_in_full(out_fd, path, length) != length)
                        die("unable to write rerere record");
        }
index c77ab710c459cb292ee2162919ed28e260e5da88..b3471e417de64fb6bef300923f4b2c1b6351fa6d 100644 (file)
@@ -553,7 +553,7 @@ static void receive_needs(void)
 
        shallow_nr = 0;
        if (debug_fd)
-               write_in_full(debug_fd, "#S\n", 3);
+               write_str_in_full(debug_fd, "#S\n");
        for (;;) {
                struct object *o;
                unsigned char sha1_buf[20];
@@ -619,7 +619,7 @@ static void receive_needs(void)
                }
        }
        if (debug_fd)
-               write_in_full(debug_fd, "#E\n", 3);
+               write_str_in_full(debug_fd, "#E\n");
 
        if (!use_sideband && daemon_mode)
                no_progress = 1;