git: replace strbuf_addstr with strbuf_addch for all strings of length 2
PATCH 1/1 git: replace strbuf_addstr with strbuf_addch for all strings of length 2
From: Seija Kijin <[email protected]>
This helps reduce overhead of calculating the length
---
git: replace strbuf_addstr with strbuf_addch for all strings of length 2
This helps reduce overhead of calculating the length
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1436%2FAtariDreams%2Fstrbuf-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1436/AtariDreams/strbuf-v1
Pull-Request: https://github.com/git/git/pull/1436
bisect.c | 2 +- builtin/am.c | 6 ++---- builtin/blame.c | 8 +++----- builtin/ls-tree.c | 2 +- diff.c | 2 +- log-tree.c | 2 +- merge-ort.c | 3 +-- path.c | 2 +- protocol-caps.c | 2 +- reftable/readwrite_test.c | 2 +- reftable/refname.c | 2 +- reftable/stack.c | 12 ++++++------ reftable/stack_test.c | 2 +- reftable/writer.c | 2 +- sequencer.c | 2 +- setup.c | 2 +- trace2/tr2_tgt_normal.c | 2 +- 17 files changed, 25 insertions(+), 30 deletions(-)
bisect.c +1 -1
@@ -443,7 +443,7 @@ static int register_ref(const char *refname, const struct object_id *oid, {struct strbuf good_prefix =STRBUF_INIT ;strbuf_addstr (&good_prefix, term_good); -strbuf_addstr (&good_prefix,"-" ); +strbuf_addch (&good_prefix,'-' );if (!strcmp (refname, term_bad)) { current_bad_oid =xmalloc (sizeof (*current_bad_oid));
builtin/am.c +2 -4
@@ -329,13 +329,11 @@ static void write_author_script(const struct am_state *state)strbuf_addstr (&sb,"GIT_AUTHOR_NAME=" );sq_quote_buf (&sb, state->author_name); -strbuf_addch (&sb,'\n' ); -strbuf_addstr (&sb,"GIT_AUTHOR_EMAIL=" ); +strbuf_addstr (&sb,"\nGIT_AUTHOR_EMAIL=" );sq_quote_buf (&sb, state->author_email); -strbuf_addch (&sb,'\n' ); -strbuf_addstr (&sb,"GIT_AUTHOR_DATE=" ); +strbuf_addstr (&sb,"\nGIT_AUTHOR_DATE=" );sq_quote_buf (&sb, state->author_date);strbuf_addch (&sb,'\n' );
builtin/blame.c +3 -5
@@ -143,11 +143,9 @@ static void get_ac_line(const char *inbuf, const char *what,if (split_ident_line (&ident, tmp, len)) { error_out: -/* Ugh */ - tmp ="(unknown)" ; -strbuf_addstr (name, tmp); -strbuf_addstr (mail, tmp); -strbuf_addstr (tz, tmp); +strbuf_addstr (name,"(unknown)" ); +strbuf_addstr (mail,"(unknown)" ); +strbuf_addstr (tz,"(unknown)" ); *time =0 ;return ; }
builtin/ls-tree.c +1 -1
@@ -60,7 +60,7 @@ static void expand_objectsize(struct strbuf *line, const struct object_id *oid, }else if (padded) {strbuf_addf (line,"%7s" ,"-" ); }else { -strbuf_addstr (line,"-" ); +strbuf_addch (line,'-' ); } }
diff.c +1 -1
@@ -1702,7 +1702,7 @@ static void add_line_count(struct strbuf *out, int count)strbuf_addstr (out,"0,0" );break ;case 1 : -strbuf_addstr (out,"1" ); +strbuf_addch (out,'1' );break ;default :strbuf_addf (out,"1,%d" , count);
log-tree.c +1 -1
@@ -388,7 +388,7 @@ void fmt_output_subject(struct strbuf *filename,strbuf_addf (&temp,"v%s" , info->reroll_count);format_sanitized_subject (filename, temp.buf, temp.len); -strbuf_addstr (filename,"-" ); +strbuf_addch (filename,'-' );strbuf_release (&temp); }strbuf_addf (filename,"%04d-%s" , nr, subject);
merge-ort.c +1 -2
@@ -801,8 +801,7 @@ static void path_msg(struct merge_options *opt,va_start (ap, fmt);if (opt->priv->call_depth) { -strbuf_addchars (dest,' ' ,2 ); -strbuf_addstr (dest,"From inner merge:" ); +strbuf_addstr (dest," From inner merge:" );strbuf_addchars (dest,' ' , opt->priv->call_depth *2 ); }strbuf_vaddf (dest, fmt, ap);
path.c +1 -1
@@ -1082,7 +1082,7 @@ const char *remove_leading_path(const char *in, const char *prefix)strbuf_reset (&buf);if (!in[j]) -strbuf_addstr (&buf,"." ); +strbuf_addch (&buf,'.' );else strbuf_addstr (&buf, in + j);return buf.buf;
protocol-caps.c +1 -1
@@ -63,7 +63,7 @@ static void send_info(struct repository *r, struct packet_writer *writer,if (info->size) {if (oid_object_info (r, &oid, &object_size) <0 ) { -strbuf_addstr (&send_buffer," " ); +strbuf_addch (&send_buffer,' ' ); }else {strbuf_addf (&send_buffer," %lu" , object_size); }
reftable/readwrite_test.c +1 -1
@@ -483,7 +483,7 @@ static void test_table_read_write_seek(int index, int hash_id) }strbuf_addstr (&pastLast, names[N -1 ]); -strbuf_addstr (&pastLast,"/" ); +strbuf_addch (&pastLast,'/' ); err =reftable_reader_seek_ref (&rd, &it, pastLast.buf);if (err ==0 ) {
reftable/refname.c +1 -1
@@ -179,7 +179,7 @@ int modification_validate(struct modification *mod) goto done;strbuf_reset (&slashed);strbuf_addstr (&slashed, mod->add[i]); -strbuf_addstr (&slashed,"/" ); +strbuf_addch (&slashed,'/' ); err =modification_has_ref_with_prefix (mod, slashed.buf);if (err ==0 ) {
reftable/stack.c +6 -6
@@ -35,7 +35,7 @@ static void stack_filename(struct strbuf *dest, struct reftable_stack *st, {strbuf_reset (dest);strbuf_addstr (dest, st->reftable_dir); -strbuf_addstr (dest,"/" ); +strbuf_addch (dest,'/' );strbuf_addstr (dest, name); }
@@ -547,11 +547,11 @@ int reftable_addition_commit(struct reftable_addition *add)for (i =0 ; i < add->stack->merged->stack_len; i++) {strbuf_addstr (&table_list, add->stack->readers[i]->name); -strbuf_addstr (&table_list,"\n" ); +strbuf_addch (&table_list,'\n' ); }for (i =0 ; i < add->new_tables_len; i++) {strbuf_addstr (&table_list, add->new_tables[i]); -strbuf_addstr (&table_list,"\n" ); +strbuf_addch (&table_list,'\n' ); } err =write (add->lock_file_fd, table_list.buf, table_list.len);
@@ -1013,15 +1013,15 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last,for (i =0 ; i < first; i++) {strbuf_addstr (&ref_list_contents, st->readers[i]->name); -strbuf_addstr (&ref_list_contents,"\n" ); +strbuf_addch (&ref_list_contents,'\n' ); }if (!is_empty_table) {strbuf_addbuf (&ref_list_contents, &new_table_name); -strbuf_addstr (&ref_list_contents,"\n" ); +strbuf_addch (&ref_list_contents,'\n' ); }for (i = last +1 ; i < st->merged->stack_len; i++) {strbuf_addstr (&ref_list_contents, st->readers[i]->name); -strbuf_addstr (&ref_list_contents,"\n" ); +strbuf_addch (&ref_list_contents,'\n' ); } err =write (lock_file_fd, ref_list_contents.buf, ref_list_contents.len);
reftable/stack_test.c +1 -1
@@ -181,7 +181,7 @@ static void test_reftable_stack_add_one(void)strbuf_reset (&scratch);strbuf_addstr (&scratch, dir); -strbuf_addstr (&scratch,"/" ); +strbuf_addch (&scratch,'/' );/* do not try at home; not an external API for reftable. */ strbuf_addstr (&scratch, st->readers[0 ]->name); err =stat (scratch.buf, &stat_result);
reftable/writer.c +1 -1
@@ -351,7 +351,7 @@ int reftable_writer_add_log(struct reftable_writer *w, err =REFTABLE_API_ERROR ; goto done; } -strbuf_addstr (&cleaned_message,"\n" ); +strbuf_addch (&cleaned_message,'\n' ); log->value.update.message = cleaned_message.buf; }
sequencer.c +1 -1
@@ -2233,7 +2233,7 @@ static int do_pick_commit(struct repository *r, }else {strbuf_addstr (&msgbuf,"Revert \"" );strbuf_addstr (&msgbuf, msg.subject); -strbuf_addstr (&msgbuf,"\"" ); +strbuf_addch (&msgbuf,'\"' ); }strbuf_addstr (&msgbuf,"\n\nThis reverts commit " );refer_to_commit (opts, &msgbuf, commit);
setup.c +1 -1
@@ -1349,7 +1349,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,return GIT_DIR_DISALLOWED_BARE ;if (!ensure_valid_ownership (NULL ,NULL , dir->buf, report))return GIT_DIR_INVALID_OWNERSHIP ; -strbuf_addstr (gitdir,"." ); +strbuf_addch (gitdir,'.' );return GIT_DIR_BARE ; }
trace2/tr2_tgt_normal.c +1 -1
@@ -224,7 +224,7 @@ static void fn_child_start_fl(const char *file, int line,if (cmd->dir) {strbuf_addstr (&buf_payload," cd " );sq_quote_buf_pretty (&buf_payload, cmd->dir); -strbuf_addstr (&buf_payload,";" ); +strbuf_addch (&buf_payload,';' ); }/*
base-commit: a7caae2729742fc80147bca1c02ae848cb55921a
Signature · 1 line
gitgitgadget
Quoted 3 lines · Rose via GitGitGadget
From: Seija Kijin <[email protected]>
This helps reduce overhead of calculating the length
Have you measured how much overhead this change is saving, or is this a 300 line e-mail message that churns code without giving us any measurable improvement?
There also seem to be some totally unrelated changes of dubious merit hidden in these patches (see below).
Quoted 1 line
Subject: Re: [PATCH] git: replace
strbuf_addstrwithstrbuf_addchfor all strings of length 2
I think you are touching strings of length 1, not 2. Running strlen() on such a string returns will return 1 without counting the terminating NUL.
Quoted 9 lines
builtin/am.c +0
-1@@ -329,13 +329,11 @@ static void write_author_script(const struct am_state *state)strbuf_addstr (&sb,"GIT_AUTHOR_NAME=" );sq_quote_buf (&sb, state->author_name); -strbuf_addch (&sb,'\n' );
builtin/am.c static void write_author_script(const struct am_state *state) · line 338
Quoted 10 lines
builtin/am.c +2
-3-strbuf_addstr (&sb,"GIT_AUTHOR_EMAIL=" ); +strbuf_addstr (&sb,"\nGIT_AUTHOR_EMAIL=" );sq_quote_buf (&sb, state->author_email); -strbuf_addch (&sb,'\n' ); -strbuf_addstr (&sb,"GIT_AUTHOR_DATE=" ); +strbuf_addstr (&sb,"\nGIT_AUTHOR_DATE=" );sq_quote_buf (&sb, state->author_date);strbuf_addch (&sb,'\n' );
This may reduce the number of lines, but markedly worsens the readability of the resulting code. Each of the three-line blocks in the original used to be logically complete and independent unit, but now each of them depend on what the last block wants.
In any case, this has nothing to do with "addstr() of constant string can be replaced with add() with a constant length or addch() of a sing character to make it unnecessary to compute the length".
By the way, the current implementation of strbuf_addstr() looks like this:
static inline void strbuf_addstr(struct strbuf *sb, const char *s)
{
strbuf_add(sb, s, strlen(s));
}Decent optimizing compilers should be able to see through the code like this you write:
strbuf_addstr(&sb, "constant");
which becomes
strbuf_add(&sb, "constant", strlen("constant"))when inlined, and realize strlen("constant") can be computed at compile time, turning it into
strbuf_add(&sb, "constant", 8);
That way, people can make their strbuf_addstr() calls in the way they find the most natural, i.e. without having to choose between _addstr() and _add(). If the compilers can do their unnecessary thinking for us humans, we should make them do so to help us.
If somebody can prove that between these two
strbuf_add(&sb, "c", 1); strbuf_addch(&sb, 'c');
there is a meaningful difference in overhead to encourage us to rewrite the former to the latter, perhaps a similar trick can be employed in the implementation of strbuf_add(), perhaps like:
static inline void strbuf_add(struct strbuf *sb, const void *data, size_t len)
{
if (len == 1) {
strbuf_addch(sb, *((char *)sb));
} else {
strbuf_grow(sb, len);
memcpy(sb->buf + sb->len, data, len);
strbuf_setlen(sb, sb->len + len);
}
}That way, people can make their strbuf_add() and strbuf_addstr() calls in the way they find the most natural, i.e. without having to choose between _add() and _addch() depending on the length of the string. This makes the code easier to maintain, as we do not have to change the code all that much when the length of the string we need to append to a strbuf changes from 1 to more (or the other way around).
But I somehow doubt it is worth it.
Quoted 6 lines
builtin/blame.c +0
-0@@ -143,11 +143,9 @@ static void get_ac_line(const char *inbuf, const char *what,
builtin/blame.c static void get_ac_line(const char *inbuf, const char *what, · line 148
Quoted 10 lines
builtin/blame.c +3
-5if (split_ident_line (&ident, tmp, len)) { error_out: -/* Ugh */ - tmp ="(unknown)" ; -strbuf_addstr (name, tmp); -strbuf_addstr (mail, tmp); -strbuf_addstr (tz, tmp); +strbuf_addstr (name,"(unknown)" ); +strbuf_addstr (mail,"(unknown)" ); +strbuf_addstr (tz,"(unknown)" );
This is another unrelated change that has not much to do with the theme of the change, to use addch() when the string is of length 1.
builtin/blame.c static void get_ac_line(const char *inbuf, const char *what,
Quoted 5 lines
builtin/blame.c +1
-2if (opt->priv->call_depth) { -strbuf_addchars (dest,' ' ,2 ); -strbuf_addstr (dest,"From inner merge:" ); +strbuf_addstr (dest," From inner merge:" );strbuf_addchars (dest,' ' , opt->priv->call_depth *2 );
Ditto, even though this is not as horrible as the change to builtin/am.c we saw earlier.
I'll stop here.
Quoted 8 lines · Junio C Hamano
Quoted 8 lines
From: Seija Kijin <[email protected]> This helps reduce overhead of calculating the length
builtin/am.c +0
-1strbuf_addstr (&sb,"GIT_AUTHOR_NAME=" );sq_quote_buf (&sb, state->author_name); -strbuf_addch (&sb,'\n' );
Quoted 7 lines
builtin/am.c static void write_author_script(const struct am_state *state) · line 333
Quoted 2 lines
builtin/am.c +1
-1-strbuf_addstr (&sb,"GIT_AUTHOR_EMAIL=" ); +strbuf_addstr (&sb,"\nGIT_AUTHOR_EMAIL=" );This may reduce the number of lines, but markedly worsens the readability of the resulting code. Each of the three-line blocks in the original used to be logically complete and independent unit, but now each of them depend on what the last block wants.
Very much agree with this and all your other review comments.
Quoted 7 lines
builtin/am.c
Quoted 4 lines
builtin/am.c +1
-2-strbuf_addchars (dest,' ' ,2 ); -strbuf_addstr (dest,"From inner merge:" ); +strbuf_addstr (dest," From inner merge:" );strbuf_addchars (dest,' ' , opt->priv->call_depth *2 );Ditto, even though this is not as horrible as the change to
builtin/am.cwe saw earlier.
Additionally, if this literal string ever gets wrapped in _(...), then the above change is even more undesirable due to the extra burden it places on translators.
PATCH v2 1/1 git: replace strbuf_addstr with strbuf_addch for all strings of length 2
In reply to Rose via GitGitGadgetFrom: Seija Kijin <[email protected]>
This helps reduce overhead of calculating the length
---
git: replace strbuf_addstr with strbuf_addch for all strings of length 2
This helps reduce overhead of calculating the length
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1436%2FAreaZR%2Fstrbuf-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1436/AreaZR/strbuf-v2
Pull-Request: https://github.com/git/git/pull/1436
Range-diff vs v1:
1: 8eed09a9df1 ! 1: 22c37ed6677 git: replace strbuf_addstr with strbuf_addch for all strings of length 2
@@ Commit message
## bisect.c ##
-@@ bisect.c: static int register_ref(const char *refname, const struct object_id *oid, +@@ bisect.c: static int register_ref(const char *refname, const char *referent UNUSED, const
{
struct strbuf good_prefix = STRBUF_INIT;
strbuf_addstr(&good_prefix, term_good);
@@ bisect.c: static int register_ref(const char *refname, const struct object_id *o
+ strbuf_addch(&good_prefix, '-');
if (!strcmp(refname, term_bad)) {
- current_bad_oid = xmalloc(sizeof(*current_bad_oid));
+ free(current_bad_oid);
## builtin/am.c ##
@@ builtin/am.c: static void write_author_script(const struct am_state *state)
@@ protocol-caps.c: static void send_info(struct repository *r, struct packet_write
strbuf_addf(&send_buffer, " %lu", object_size);
}
- ## reftable/readwrite_test.c ##
-@@ reftable/readwrite_test.c: static void test_table_read_write_seek(int index, int hash_id)
- }
-
- strbuf_addstr(&pastLast, names[N - 1]);
-- strbuf_addstr(&pastLast, "/");
-+ strbuf_addch(&pastLast, '/');
-
- err = reftable_reader_seek_ref(&rd, &it, pastLast.buf);
- if (err == 0) {
-
- ## reftable/refname.c ##
-@@ reftable/refname.c: int modification_validate(struct modification *mod)
- goto done;
- strbuf_reset(&slashed);
- strbuf_addstr(&slashed, mod->add[i]);
-- strbuf_addstr(&slashed, "/");
-+ strbuf_addch(&slashed, '/');
-
- err = modification_has_ref_with_prefix(mod, slashed.buf);
- if (err == 0) {
-
- ## reftable/stack.c ##
-@@ reftable/stack.c: static void stack_filename(struct strbuf *dest, struct reftable_stack *st,
- {
- strbuf_reset(dest);
- strbuf_addstr(dest, st->reftable_dir);
-- strbuf_addstr(dest, "/");
-+ strbuf_addch(dest, '/');
- strbuf_addstr(dest, name);
- }
-
-@@ reftable/stack.c: int reftable_addition_commit(struct reftable_addition *add)
-
- for (i = 0; i < add->stack->merged->stack_len; i++) {
- strbuf_addstr(&table_list, add->stack->readers[i]->name);
-- strbuf_addstr(&table_list, "\n");
-+ strbuf_addch(&table_list, '\n');
- }
- for (i = 0; i < add->new_tables_len; i++) {
- strbuf_addstr(&table_list, add->new_tables[i]);
-- strbuf_addstr(&table_list, "\n");
-+ strbuf_addch(&table_list, '\n');
- }
-
- err = write(add->lock_file_fd, table_list.buf, table_list.len);
-@@ reftable/stack.c: static int stack_compact_range(struct reftable_stack *st, int first, int last, -
- for (i = 0; i < first; i++) {
- strbuf_addstr(&ref_list_contents, st->readers[i]->name);
-- strbuf_addstr(&ref_list_contents, "\n");
-+ strbuf_addch(&ref_list_contents, '\n');
- }
- if (!is_empty_table) {
- strbuf_addbuf(&ref_list_contents, &new_table_name);
-- strbuf_addstr(&ref_list_contents, "\n");
-+ strbuf_addch(&ref_list_contents, '\n');
- }
- for (i = last + 1; i < st->merged->stack_len; i++) {
- strbuf_addstr(&ref_list_contents, st->readers[i]->name);
-- strbuf_addstr(&ref_list_contents, "\n");
-+ strbuf_addch(&ref_list_contents, '\n');
- }
-
- err = write(lock_file_fd, ref_list_contents.buf, ref_list_contents.len);
-
- ## reftable/stack_test.c ##
-@@ reftable/stack_test.c: static void test_reftable_stack_add_one(void)
-
- strbuf_reset(&scratch);
- strbuf_addstr(&scratch, dir);
-- strbuf_addstr(&scratch, "/");
-+ strbuf_addch(&scratch, '/');
- /* do not try at home; not an external API for reftable. */
- strbuf_addstr(&scratch, st->readers[0]->name);
- err = stat(scratch.buf, &stat_result);
-
- ## reftable/writer.c ##
-@@ reftable/writer.c: int reftable_writer_add_log(struct reftable_writer *w,
- err = REFTABLE_API_ERROR;
- goto done;
- }
-- strbuf_addstr(&cleaned_message, "\n");
-+ strbuf_addch(&cleaned_message, '\n');
- log->value.update.message = cleaned_message.buf;
- }
-
-
- ## sequencer.c ##
-@@ sequencer.c: static int do_pick_commit(struct repository *r,
- } else {
- strbuf_addstr(&msgbuf, "Revert \"");
- strbuf_addstr(&msgbuf, msg.subject);
-- strbuf_addstr(&msgbuf, "\"");
-+ strbuf_addch(&msgbuf, '\"');
- }
- strbuf_addstr(&msgbuf, "\n\nThis reverts commit ");
- refer_to_commit(opts, &msgbuf, commit);
-
## setup.c ##
@@ setup.c: static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
return GIT_DIR_DISALLOWED_BARE;
bisect.c | 2 +- builtin/am.c | 6 ++---- builtin/blame.c | 8 +++----- builtin/ls-tree.c | 2 +- diff.c | 2 +- log-tree.c | 2 +- merge-ort.c | 3 +-- path.c | 2 +- protocol-caps.c | 2 +- setup.c | 2 +- trace2/tr2_tgt_normal.c | 2 +- 11 files changed, 14 insertions(+), 19 deletions(-)
bisect.c +1 -1
@@ -454,7 +454,7 @@ static int register_ref(const char *refname, const char *referent UNUSED, const {struct strbuf good_prefix =STRBUF_INIT ;strbuf_addstr (&good_prefix, term_good); -strbuf_addstr (&good_prefix,"-" ); +strbuf_addch (&good_prefix,'-' );if (!strcmp (refname, term_bad)) {free (current_bad_oid);
builtin/am.c +2 -4
@@ -341,13 +341,11 @@ static void write_author_script(const struct am_state *state)strbuf_addstr (&sb,"GIT_AUTHOR_NAME=" );sq_quote_buf (&sb, state->author_name); -strbuf_addch (&sb,'\n' ); -strbuf_addstr (&sb,"GIT_AUTHOR_EMAIL=" ); +strbuf_addstr (&sb,"\nGIT_AUTHOR_EMAIL=" );sq_quote_buf (&sb, state->author_email); -strbuf_addch (&sb,'\n' ); -strbuf_addstr (&sb,"GIT_AUTHOR_DATE=" ); +strbuf_addstr (&sb,"\nGIT_AUTHOR_DATE=" );sq_quote_buf (&sb, state->author_date);strbuf_addch (&sb,'\n' );
builtin/blame.c +3 -5
@@ -148,11 +148,9 @@ static void get_ac_line(const char *inbuf, const char *what,if (split_ident_line (&ident, tmp, len)) { error_out: -/* Ugh */ - tmp ="(unknown)" ; -strbuf_addstr (name, tmp); -strbuf_addstr (mail, tmp); -strbuf_addstr (tz, tmp); +strbuf_addstr (name,"(unknown)" ); +strbuf_addstr (mail,"(unknown)" ); +strbuf_addstr (tz,"(unknown)" ); *time =0 ;return ; }
builtin/ls-tree.c +1 -1
@@ -37,7 +37,7 @@ static void expand_objectsize(struct strbuf *line, const struct object_id *oid, }else if (padded) {strbuf_addf (line,"%7s" ,"-" ); }else { -strbuf_addstr (line,"-" ); +strbuf_addch (line,'-' ); } }
diff.c +1 -1
@@ -1763,7 +1763,7 @@ static void add_line_count(struct strbuf *out, int count)strbuf_addstr (out,"0,0" );break ;case 1 : -strbuf_addstr (out,"1" ); +strbuf_addch (out,'1' );break ;default :strbuf_addf (out,"1,%d" , count);
log-tree.c +1 -1
@@ -452,7 +452,7 @@ void fmt_output_subject(struct strbuf *filename,strbuf_addf (&temp,"v%s" , info->reroll_count);format_sanitized_subject (filename, temp.buf, temp.len); -strbuf_addstr (filename,"-" ); +strbuf_addch (filename,'-' );strbuf_release (&temp); }strbuf_addf (filename,"%04d-%s" , nr, subject);
merge-ort.c +1 -2
@@ -827,8 +827,7 @@ static void path_msg(struct merge_options *opt,va_start (ap, fmt);if (opt->priv->call_depth) { -strbuf_addchars (dest,' ' ,2 ); -strbuf_addstr (dest,"From inner merge:" ); +strbuf_addstr (dest," From inner merge:" );strbuf_addchars (dest,' ' , opt->priv->call_depth *2 ); }strbuf_vaddf (dest, fmt, ap);
path.c +1 -1
@@ -982,7 +982,7 @@ const char *remove_leading_path(const char *in, const char *prefix)strbuf_reset (&buf);if (!in[j]) -strbuf_addstr (&buf,"." ); +strbuf_addch (&buf,'.' );else strbuf_addstr (&buf, in + j);return buf.buf;
protocol-caps.c +1 -1
@@ -65,7 +65,7 @@ static void send_info(struct repository *r, struct packet_writer *writer,if (info->size) {if (oid_object_info (r, &oid, &object_size) <0 ) { -strbuf_addstr (&send_buffer," " ); +strbuf_addch (&send_buffer,' ' ); }else {strbuf_addf (&send_buffer," %lu" , object_size); }
setup.c +1 -1
@@ -1550,7 +1550,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,return GIT_DIR_DISALLOWED_BARE ;if (!ensure_valid_ownership (NULL ,NULL , dir->buf, report))return GIT_DIR_INVALID_OWNERSHIP ; -strbuf_addstr (gitdir,"." ); +strbuf_addch (gitdir,'.' );return GIT_DIR_BARE ; }
trace2/tr2_tgt_normal.c +1 -1
@@ -226,7 +226,7 @@ static void fn_child_start_fl(const char *file, int line,if (cmd->dir) {strbuf_addstr (&buf_payload," cd " );sq_quote_buf_pretty (&buf_payload, cmd->dir); -strbuf_addstr (&buf_payload,";" ); +strbuf_addch (&buf_payload,';' ); }/*
base-commit: 2ccc89b0c16c51561da90d21cfbb4b58cc877bf6
Signature · 1 line
gitgitgadget
PATCH v3 1/1 git: replace strbuf_addstr with strbuf_addch for all strings of length 2
From: Seija Kijin <[email protected]>
Adding the char directly instead of a string of length 2 is clearer and more efficient.
---
git: replace strbuf_addstr with strbuf_addch for all strings of length 2
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1436%2FAreaZR%2Fstrbuf-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1436/AreaZR/strbuf-v3
Pull-Request: https://github.com/git/git/pull/1436
Range-diff vs v2:
1: 22c37ed6677 ! 1: 96a093dec36 git: replace strbuf_addstr with strbuf_addch for all strings of length 2
@@ Metadata
## Commit message ##
git: replace strbuf_addstr with strbuf_addch for all strings of length 2
- This helps reduce overhead of calculating the length
+ Adding the char directly instead of a string of length 2
+ is clearer and more efficient.
@@ log-tree.c: void fmt_output_subject(struct strbuf *filename,
}
strbuf_addf(filename, "%04d-%s", nr, subject);
- ## merge-ort.c ##
-@@ merge-ort.c: static void path_msg(struct merge_options *opt, -
- va_start(ap, fmt);
- if (opt->priv->call_depth) {
-- strbuf_addchars(dest, ' ', 2);
-- strbuf_addstr(dest, "From inner merge:");
-+ strbuf_addstr(dest, " From inner merge:");
- strbuf_addchars(dest, ' ', opt->priv->call_depth * 2);
- }
- strbuf_vaddf(dest, fmt, ap);
-
## path.c ##
@@ path.c: const char *remove_leading_path(const char *in, const char *prefix)
bisect.c | 2 +- builtin/am.c | 6 ++---- builtin/blame.c | 8 +++----- builtin/ls-tree.c | 2 +- diff.c | 2 +- log-tree.c | 2 +- path.c | 2 +- protocol-caps.c | 2 +- setup.c | 2 +- trace2/tr2_tgt_normal.c | 2 +- 10 files changed, 13 insertions(+), 17 deletions(-)
bisect.c +1 -1
@@ -454,7 +454,7 @@ static int register_ref(const char *refname, const char *referent UNUSED, const {struct strbuf good_prefix =STRBUF_INIT ;strbuf_addstr (&good_prefix, term_good); -strbuf_addstr (&good_prefix,"-" ); +strbuf_addch (&good_prefix,'-' );if (!strcmp (refname, term_bad)) {free (current_bad_oid);
builtin/am.c +2 -4
@@ -341,13 +341,11 @@ static void write_author_script(const struct am_state *state)strbuf_addstr (&sb,"GIT_AUTHOR_NAME=" );sq_quote_buf (&sb, state->author_name); -strbuf_addch (&sb,'\n' ); -strbuf_addstr (&sb,"GIT_AUTHOR_EMAIL=" ); +strbuf_addstr (&sb,"\nGIT_AUTHOR_EMAIL=" );sq_quote_buf (&sb, state->author_email); -strbuf_addch (&sb,'\n' ); -strbuf_addstr (&sb,"GIT_AUTHOR_DATE=" ); +strbuf_addstr (&sb,"\nGIT_AUTHOR_DATE=" );sq_quote_buf (&sb, state->author_date);strbuf_addch (&sb,'\n' );
builtin/blame.c +3 -5
@@ -148,11 +148,9 @@ static void get_ac_line(const char *inbuf, const char *what,if (split_ident_line (&ident, tmp, len)) { error_out: -/* Ugh */ - tmp ="(unknown)" ; -strbuf_addstr (name, tmp); -strbuf_addstr (mail, tmp); -strbuf_addstr (tz, tmp); +strbuf_addstr (name,"(unknown)" ); +strbuf_addstr (mail,"(unknown)" ); +strbuf_addstr (tz,"(unknown)" ); *time =0 ;return ; }
builtin/ls-tree.c +1 -1
@@ -37,7 +37,7 @@ static void expand_objectsize(struct strbuf *line, const struct object_id *oid, }else if (padded) {strbuf_addf (line,"%7s" ,"-" ); }else { -strbuf_addstr (line,"-" ); +strbuf_addch (line,'-' ); } }
diff.c +1 -1
@@ -1763,7 +1763,7 @@ static void add_line_count(struct strbuf *out, int count)strbuf_addstr (out,"0,0" );break ;case 1 : -strbuf_addstr (out,"1" ); +strbuf_addch (out,'1' );break ;default :strbuf_addf (out,"1,%d" , count);
log-tree.c +1 -1
@@ -452,7 +452,7 @@ void fmt_output_subject(struct strbuf *filename,strbuf_addf (&temp,"v%s" , info->reroll_count);format_sanitized_subject (filename, temp.buf, temp.len); -strbuf_addstr (filename,"-" ); +strbuf_addch (filename,'-' );strbuf_release (&temp); }strbuf_addf (filename,"%04d-%s" , nr, subject);
path.c +1 -1
@@ -982,7 +982,7 @@ const char *remove_leading_path(const char *in, const char *prefix)strbuf_reset (&buf);if (!in[j]) -strbuf_addstr (&buf,"." ); +strbuf_addch (&buf,'.' );else strbuf_addstr (&buf, in + j);return buf.buf;
protocol-caps.c +1 -1
@@ -65,7 +65,7 @@ static void send_info(struct repository *r, struct packet_writer *writer,if (info->size) {if (oid_object_info (r, &oid, &object_size) <0 ) { -strbuf_addstr (&send_buffer," " ); +strbuf_addch (&send_buffer,' ' ); }else {strbuf_addf (&send_buffer," %lu" , object_size); }
setup.c +1 -1
@@ -1550,7 +1550,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,return GIT_DIR_DISALLOWED_BARE ;if (!ensure_valid_ownership (NULL ,NULL , dir->buf, report))return GIT_DIR_INVALID_OWNERSHIP ; -strbuf_addstr (gitdir,"." ); +strbuf_addch (gitdir,'.' );return GIT_DIR_BARE ; }
trace2/tr2_tgt_normal.c +1 -1
@@ -226,7 +226,7 @@ static void fn_child_start_fl(const char *file, int line,if (cmd->dir) {strbuf_addstr (&buf_payload," cd " );sq_quote_buf_pretty (&buf_payload, cmd->dir); -strbuf_addstr (&buf_payload,";" ); +strbuf_addch (&buf_payload,';' ); }/*
base-commit: d882f382b3d939d90cfa58d17b17802338f05d66
Signature · 1 line
gitgitgadget
PATCH v4 1/1 git: replace strbuf_addstr with strbuf_addch for all strings of length 2
From: Seija Kijin <[email protected]>
Adding the char directly instead of a string of length 2 is clearer and more efficient.
---
git: replace strbuf_addstr with strbuf_addch for all strings of length 2
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1436%2FAreaZR%2Fstrbuf-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1436/AreaZR/strbuf-v4
Pull-Request: https://github.com/git/git/pull/1436
Range-diff vs v3:
1: 96a093dec36 ! 1: 413c5ab1f1b git: replace strbuf_addstr with strbuf_addch for all strings of length 2
@@ bisect.c: static int register_ref(const char *refname, const char *referent UNUS
if (!strcmp(refname, term_bad)) {
free(current_bad_oid);
- ## builtin/am.c ##
-@@ builtin/am.c: static void write_author_script(const struct am_state *state)
-
- strbuf_addstr(&sb, "GIT_AUTHOR_NAME=");
- sq_quote_buf(&sb, state->author_name);
-- strbuf_addch(&sb, '\n');
-
-- strbuf_addstr(&sb, "GIT_AUTHOR_EMAIL=");
-+ strbuf_addstr(&sb, "\nGIT_AUTHOR_EMAIL=");
- sq_quote_buf(&sb, state->author_email);
-- strbuf_addch(&sb, '\n');
-
-- strbuf_addstr(&sb, "GIT_AUTHOR_DATE=");
-+ strbuf_addstr(&sb, "\nGIT_AUTHOR_DATE=");
- sq_quote_buf(&sb, state->author_date);
- strbuf_addch(&sb, '\n');
-
-
- ## builtin/blame.c ##
-@@ builtin/blame.c: static void get_ac_line(const char *inbuf, const char *what,
-
- if (split_ident_line(&ident, tmp, len)) {
- error_out:
-- /* Ugh */
-- tmp = "(unknown)";
-- strbuf_addstr(name, tmp);
-- strbuf_addstr(mail, tmp);
-- strbuf_addstr(tz, tmp);
-+ strbuf_addstr(name, "(unknown)");
-+ strbuf_addstr(mail, "(unknown)");
-+ strbuf_addstr(tz, "(unknown)");
- *time = 0;
- return;
- }
-
## builtin/ls-tree.c ##
@@ builtin/ls-tree.c: static void expand_objectsize(struct strbuf *line, const struct object_id *oid,
} else if (padded) {
@@ builtin/ls-tree.c: static void expand_objectsize(struct strbuf *line, const stru
}
+ ## convert.c ##
+@@ convert.c: static void trace_encoding(const char *context, const char *path,
+ ((i+1) % 8 && (i+1) < len ? ' ' : '\n')
+ );
+ }
+- strbuf_addchars(&trace, '\n', 1);
++ strbuf_addch(&trace, '\n');
+
+ trace_strbuf(&coe, &trace);
+ strbuf_release(&trace);
+
## diff.c ##
@@ diff.c: static void add_line_count(struct strbuf *out, int count)
strbuf_addstr(out, "0,0");
@@ log-tree.c: void fmt_output_subject(struct strbuf *filename,
}
strbuf_addf(filename, "%04d-%s", nr, subject);
+ ## merge-ort.c ##
+@@ merge-ort.c: static void path_msg(struct merge_options *opt,
+
+ va_start(ap, fmt);
+ if (opt->priv->call_depth) {
+- strbuf_addchars(dest, ' ', 2);
++ strbuf_addstr(dest, " ");
+ strbuf_addstr(dest, "From inner merge:");
+ strbuf_addchars(dest, ' ', opt->priv->call_depth * 2);
+ }
+
## path.c ##
@@ path.c: const char *remove_leading_path(const char *in, const char *prefix)
@@ path.c: const char *remove_leading_path(const char *in, const char *prefix)
strbuf_addstr(&buf, in + j);
return buf.buf;
+ ## pretty.c ##
+@@ pretty.c: void pp_user_info(struct pretty_print_context *pp,
+
+ strbuf_addf(sb, "%s: ", what);
+ if (pp->fmt == CMIT_FMT_FULLER)
+- strbuf_addchars(sb, ' ', 4);
++ strbuf_addstr(sb, " ");
+
+ strbuf_addf(&id, "%.*s <%.*s>", (int)namelen, namebuf,
+ (int)maillen, mailbuf);
+
## protocol-caps.c ##
@@ protocol-caps.c: static void send_info(struct repository *r, struct packet_writer *writer,
@@ protocol-caps.c: static void send_info(struct repository *r, struct packet_write
strbuf_addf(&send_buffer, " %lu", object_size);
}
+ ## send-pack.c ##
+@@ send-pack.c: static int generate_push_cert(struct strbuf *req_buf,
+ if (args->push_options)
+ for_each_string_list_item(item, args->push_options)
+ strbuf_addf(&cert, "push-option %s\n", item->string);
+- strbuf_addstr(&cert, "\n");
++ strbuf_addch(&cert, '\n');
+
+ for (ref = remote_refs; ref; ref = ref->next) {
+ if (check_to_send_update(ref, args) < 0)
+
## setup.c ##
@@ setup.c: static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
return GIT_DIR_DISALLOWED_BARE;bisect.c | 2 +- builtin/ls-tree.c | 2 +- convert.c | 2 +- diff.c | 2 +- log-tree.c | 2 +- merge-ort.c | 2 +- path.c | 2 +- pretty.c | 2 +- protocol-caps.c | 2 +- send-pack.c | 2 +- setup.c | 2 +- trace2/tr2_tgt_normal.c | 2 +- 12 files changed, 12 insertions(+), 12 deletions(-)
bisect.c +1 -1
@@ -454,7 +454,7 @@ static int register_ref(const char *refname, const char *referent UNUSED, const {struct strbuf good_prefix =STRBUF_INIT ;strbuf_addstr (&good_prefix, term_good); -strbuf_addstr (&good_prefix,"-" ); +strbuf_addch (&good_prefix,'-' );if (!strcmp (refname, term_bad)) {free (current_bad_oid);
builtin/ls-tree.c +1 -1
@@ -37,7 +37,7 @@ static void expand_objectsize(struct strbuf *line, const struct object_id *oid, }else if (padded) {strbuf_addf (line,"%7s" ,"-" ); }else { -strbuf_addstr (line,"-" ); +strbuf_addch (line,'-' ); } }
convert.c +1 -1
@@ -337,7 +337,7 @@ static void trace_encoding(const char *context, const char *path, ((i+1 ) %8 && (i+1 ) < len ?' ' :'\n' ) ); } -strbuf_addchars (&trace,'\n' ,1 ); +strbuf_addch (&trace,'\n' );trace_strbuf (&coe, &trace);strbuf_release (&trace);
diff.c +1 -1
@@ -1763,7 +1763,7 @@ static void add_line_count(struct strbuf *out, int count)strbuf_addstr (out,"0,0" );break ;case 1 : -strbuf_addstr (out,"1" ); +strbuf_addch (out,'1' );break ;default :strbuf_addf (out,"1,%d" , count);
log-tree.c +1 -1
@@ -452,7 +452,7 @@ void fmt_output_subject(struct strbuf *filename,strbuf_addf (&temp,"v%s" , info->reroll_count);format_sanitized_subject (filename, temp.buf, temp.len); -strbuf_addstr (filename,"-" ); +strbuf_addch (filename,'-' );strbuf_release (&temp); }strbuf_addf (filename,"%04d-%s" , nr, subject);
merge-ort.c +1 -1
@@ -827,7 +827,7 @@ static void path_msg(struct merge_options *opt,va_start (ap, fmt);if (opt->priv->call_depth) { -strbuf_addchars (dest,' ' ,2 ); +strbuf_addstr (dest," " );strbuf_addstr (dest,"From inner merge:" );strbuf_addchars (dest,' ' , opt->priv->call_depth *2 ); }
path.c +1 -1
@@ -982,7 +982,7 @@ const char *remove_leading_path(const char *in, const char *prefix)strbuf_reset (&buf);if (!in[j]) -strbuf_addstr (&buf,"." ); +strbuf_addch (&buf,'.' );else strbuf_addstr (&buf, in + j);return buf.buf;
pretty.c +1 -1
@@ -590,7 +590,7 @@ void pp_user_info(struct pretty_print_context *pp,strbuf_addf (sb,"%s: " , what);if (pp->fmt ==CMIT_FMT_FULLER ) -strbuf_addchars (sb,' ' ,4 ); +strbuf_addstr (sb," " );strbuf_addf (&id,"%.*s <%.*s>" , (int )namelen, namebuf, (int )maillen, mailbuf);
protocol-caps.c +1 -1
@@ -65,7 +65,7 @@ static void send_info(struct repository *r, struct packet_writer *writer,if (info->size) {if (oid_object_info (r, &oid, &object_size) <0 ) { -strbuf_addstr (&send_buffer," " ); +strbuf_addch (&send_buffer,' ' ); }else {strbuf_addf (&send_buffer," %lu" , object_size); }
send-pack.c +1 -1
@@ -373,7 +373,7 @@ static int generate_push_cert(struct strbuf *req_buf,if (args->push_options)for_each_string_list_item (item, args->push_options)strbuf_addf (&cert,"push-option %s\n" , item->string); -strbuf_addstr (&cert,"\n" ); +strbuf_addch (&cert,'\n' );for (ref = remote_refs; ref; ref = ref->next) {if (check_to_send_update (ref, args) <0 )
setup.c +1 -1
@@ -1550,7 +1550,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,return GIT_DIR_DISALLOWED_BARE ;if (!ensure_valid_ownership (NULL ,NULL , dir->buf, report))return GIT_DIR_INVALID_OWNERSHIP ; -strbuf_addstr (gitdir,"." ); +strbuf_addch (gitdir,'.' );return GIT_DIR_BARE ; }
trace2/tr2_tgt_normal.c +1 -1
@@ -226,7 +226,7 @@ static void fn_child_start_fl(const char *file, int line,if (cmd->dir) {strbuf_addstr (&buf_payload," cd " );sq_quote_buf_pretty (&buf_payload, cmd->dir); -strbuf_addstr (&buf_payload,";" ); +strbuf_addch (&buf_payload,';' ); }/*
base-commit: d882f382b3d939d90cfa58d17b17802338f05d66
Signature · 1 line
gitgitgadget