# [PATCH 2/9] builtin-apply: use warning() instead of fprintf(stderr, "warning: ") 16 messages from 2009-03-24 to 2009-03-24. Participants: Miklos Vajna, Erik Faye-Lund, Jeff King, Junio C Hamano. Thread: https://gitlist.dev/t/18499 ## Miklos Vajna, 2009-03-24 01:09 Subject: [PATCH 0/9] fprintf(stderr, "warning: ") -> warning() patches Message-ID: URL: https://gitlist.dev/e/cover.1237856682.git.vmiklos%40frugalware.org In-Reply-To: <7v3ad5dxt5.fsf@gitster.siamese.dyndns.org> ``` On Sun, Mar 22, 2009 at 02:58:46PM -0700, Junio C Hamano wrote: > > Should I rebase the series against current master and resend? > > Yes, please, if the overlap with "diff master next" is small enough. Okay, here it is, applies on top of master. All, except the http-push one, applies cleanly on top of next as well here. Miklos Vajna (9): http-push: using error() and warning() as appropriate builtin-apply: use warning() instead of fprintf(stderr, "warning: ") builtin-checkout: use warning() instead of fprintf(stderr, "warning: ") builtin-fetch-pack: use warning() instead of fprintf(stderr, "warning: ") builtin-init-db: use warning() instead of fprintf(stderr, "warning: ") builtin-rm: use warning() instead of fprintf(stderr, "warning: ") builtin-show-branch: use warning() instead of fprintf(stderr, "warning: ") builtin-show-ref: use warning() instead of fprintf(stderr, "warning: ") refs: use warning() instead of fprintf(stderr, "warning: ") builtin-apply.c | 15 +++++++-------- builtin-checkout.c | 4 ++-- builtin-fetch-pack.c | 2 +- builtin-init-db.c | 7 +++---- builtin-rm.c | 3 +-- builtin-show-branch.c | 3 +-- builtin-show-ref.c | 2 +- http-push.c | 6 +++--- refs.c | 8 +++----- 9 files changed, 22 insertions(+), 28 deletions(-) ``` ## Miklos Vajna, 2009-03-24 01:09 Subject: [PATCH 1/9] http-push: using error() and warning() as appropriate Message-ID: <1fcc4848dd7a24f846bb302bbb555bf1bac795a5.1237856682.git.vmiklos@frugalware.org> URL: https://gitlist.dev/e/1fcc4848dd7a24f846bb302bbb555bf1bac795a5.1237856682.git.vmiklos%40frugalware.org In-Reply-To: ``` Change three occurences of using inconsistent error/warning reporting by using the relevant error() / warning() calls to be consitent with the rest of the code. Signed-off-by: Miklos Vajna --- http-push.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/http-push.c b/http-push.c index 48e5f38..e6bd01a 100644 --- a/http-push.c +++ b/http-push.c @@ -759,7 +759,7 @@ static void finish_request(struct transfer_request *request) } } else { if (request->http_code == 416) - fprintf(stderr, "Warning: requested range invalid; we may already have all the data.\n"); + warning("requested range invalid; we may already have all the data."); git_inflate_end(&request->stream); git_SHA1_Final(request->real_sha1, &request->c); @@ -1616,7 +1616,7 @@ static int locking_available(void) } XML_ParserFree(parser); if (!lock_flags) - error("Error: no DAV locking support on %s", + error("no DAV locking support on %s", remote->url); } else { @@ -2225,7 +2225,7 @@ int main(int argc, char **argv) if (info_ref_lock) remote->can_update_info_refs = 1; else { - fprintf(stderr, "Error: cannot lock existing info/refs\n"); + error("cannot lock existing info/refs"); rc = 1; goto cleanup; } -- 1.6.2 ``` ## Miklos Vajna, 2009-03-24 01:09 Subject: [PATCH 2/9] builtin-apply: use warning() instead of fprintf(stderr, "warning: ") Message-ID: <9287282d966904f06ae4b3d7cfdf1d79eb645b69.1237856682.git.vmiklos@frugalware.org> URL: https://gitlist.dev/e/9287282d966904f06ae4b3d7cfdf1d79eb645b69.1237856682.git.vmiklos%40frugalware.org In-Reply-To: <1fcc4848dd7a24f846bb302bbb555bf1bac795a5.1237856682.git.vmiklos@frugalware.org> ``` Signed-off-by: Miklos Vajna --- builtin-apply.c | 15 +++++++-------- 1 files changed, 7 insertions(+), 8 deletions(-) diff --git a/builtin-apply.c b/builtin-apply.c index b52aa20..1926cd8 100644 --- a/builtin-apply.c +++ b/builtin-apply.c @@ -2451,7 +2451,7 @@ static int check_preimage(struct patch *patch, struct cache_entry **ce, struct s if ((st_mode ^ patch->old_mode) & S_IFMT) return error("%s: wrong type", old_name); if (st_mode != patch->old_mode) - fprintf(stderr, "warning: %s has type %o, expected %o\n", + warning("%s has type %o, expected %o", old_name, st_mode, patch->old_mode); if (!patch->new_mode && !patch->is_delete) patch->new_mode = st_mode; @@ -2932,8 +2932,7 @@ static int write_out_one_reject(struct patch *patch) cnt = strlen(patch->new_name); if (ARRAY_SIZE(namebuf) <= cnt + 5) { cnt = ARRAY_SIZE(namebuf) - 5; - fprintf(stderr, - "warning: truncating .rej filename to %.*s.rej", + warning("truncating .rej filename to %.*s.rej", cnt - 1, patch->new_name); } memcpy(namebuf, patch->new_name, cnt); @@ -3315,8 +3314,8 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix) squelch_whitespace_errors < whitespace_error) { int squelched = whitespace_error - squelch_whitespace_errors; - fprintf(stderr, "warning: squelched %d " - "whitespace error%s\n", + warning("squelched %d " + "whitespace error%s", squelched, squelched == 1 ? "" : "s"); } @@ -3326,12 +3325,12 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix) whitespace_error == 1 ? "" : "s", whitespace_error == 1 ? "s" : ""); if (applied_after_fixing_ws && apply) - fprintf(stderr, "warning: %d line%s applied after" - " fixing whitespace errors.\n", + warning("%d line%s applied after" + " fixing whitespace errors.", applied_after_fixing_ws, applied_after_fixing_ws == 1 ? "" : "s"); else if (whitespace_error) - fprintf(stderr, "warning: %d line%s add%s whitespace errors.\n", + warning("%d line%s add%s whitespace errors.", whitespace_error, whitespace_error == 1 ? "" : "s", whitespace_error == 1 ? "s" : ""); -- 1.6.2 ``` ## Miklos Vajna, 2009-03-24 01:09 Subject: [PATCH 3/9] builtin-checkout: use warning() instead of fprintf(stderr, "warning: ") Message-ID: URL: https://gitlist.dev/e/a1165f8583c71de7fb053e83ddf1d7acd2e48024.1237856682.git.vmiklos%40frugalware.org In-Reply-To: <9287282d966904f06ae4b3d7cfdf1d79eb645b69.1237856682.git.vmiklos@frugalware.org> ``` Signed-off-by: Miklos Vajna --- builtin-checkout.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin-checkout.c b/builtin-checkout.c index 9fdfc58..fc55bbe 100644 --- a/builtin-checkout.c +++ b/builtin-checkout.c @@ -558,8 +558,8 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new) if (!old.commit && !opts->force) { if (!opts->quiet) { - fprintf(stderr, "warning: You appear to be on a branch yet to be born.\n"); - fprintf(stderr, "warning: Forcing checkout of %s.\n", new->name); + warning("You appear to be on a branch yet to be born."); + warning("Forcing checkout of %s.", new->name); } opts->force = 1; } -- 1.6.2 ``` ## Miklos Vajna, 2009-03-24 01:09 Subject: [PATCH 4/9] builtin-fetch-pack: use warning() instead of fprintf(stderr, "warning: ") Message-ID: URL: https://gitlist.dev/e/d499d514cd704edfc54ad62422e77aca275c9527.1237856682.git.vmiklos%40frugalware.org In-Reply-To: ``` --- builtin-fetch-pack.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c index d571253..5d134be 100644 --- a/builtin-fetch-pack.c +++ b/builtin-fetch-pack.c @@ -605,7 +605,7 @@ static struct ref *do_fetch_pack(int fd[2], /* When cloning, it is not unusual to have * no common commit. */ - fprintf(stderr, "warning: no common commits\n"); + warning("no common commits"); if (get_pack(fd, pack_lockfile)) die("git fetch-pack: fetch failed."); -- 1.6.2 ``` ## Miklos Vajna, 2009-03-24 01:09 Subject: [PATCH 5/9] builtin-init-db: use warning() instead of fprintf(stderr, "warning: ") Message-ID: <6ca31ce7256d8945f520b7d57e2a14b890731abd.1237856682.git.vmiklos@frugalware.org> URL: https://gitlist.dev/e/6ca31ce7256d8945f520b7d57e2a14b890731abd.1237856682.git.vmiklos%40frugalware.org In-Reply-To: ``` --- builtin-init-db.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-) diff --git a/builtin-init-db.c b/builtin-init-db.c index ee3911f..fc63d0f 100644 --- a/builtin-init-db.c +++ b/builtin-init-db.c @@ -130,8 +130,7 @@ static void copy_templates(const char *template_dir) } dir = opendir(template_path); if (!dir) { - fprintf(stderr, "warning: templates not found %s\n", - template_dir); + warning("templates not found %s", template_dir); return; } @@ -144,8 +143,8 @@ static void copy_templates(const char *template_dir) if (repository_format_version && repository_format_version != GIT_REPO_VERSION) { - fprintf(stderr, "warning: not copying templates of " - "a wrong format version %d from '%s'\n", + warning("not copying templates of " + "a wrong format version %d from '%s'", repository_format_version, template_dir); closedir(dir); -- 1.6.2 ``` ## Miklos Vajna, 2009-03-24 01:09 Subject: [PATCH 6/9] builtin-rm: use warning() instead of fprintf(stderr, "warning: ") Message-ID: <83e5caf5764a146779d8cefbf874f540a6160eb4.1237856682.git.vmiklos@frugalware.org> URL: https://gitlist.dev/e/83e5caf5764a146779d8cefbf874f540a6160eb4.1237856682.git.vmiklos%40frugalware.org In-Reply-To: <6ca31ce7256d8945f520b7d57e2a14b890731abd.1237856682.git.vmiklos@frugalware.org> ``` Signed-off-by: Miklos Vajna --- builtin-rm.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/builtin-rm.c b/builtin-rm.c index c11f455..269d608 100644 --- a/builtin-rm.c +++ b/builtin-rm.c @@ -59,8 +59,7 @@ static int check_local_mod(unsigned char *head, int index_only) if (lstat(ce->name, &st) < 0) { if (errno != ENOENT) - fprintf(stderr, "warning: '%s': %s", - ce->name, strerror(errno)); + warning("'%s': %s", ce->name, strerror(errno)); /* It already vanished from the working tree */ continue; } -- 1.6.2 ``` ## Miklos Vajna, 2009-03-24 01:09 Subject: [PATCH 7/9] builtin-show-branch: use warning() instead of fprintf(stderr, "warning: ") Message-ID: <5e3412be5d72bce2bf36fdcc2734b89386fdfc21.1237856682.git.vmiklos@frugalware.org> URL: https://gitlist.dev/e/5e3412be5d72bce2bf36fdcc2734b89386fdfc21.1237856682.git.vmiklos%40frugalware.org In-Reply-To: <83e5caf5764a146779d8cefbf874f540a6160eb4.1237856682.git.vmiklos@frugalware.org> ``` Signed-off-by: Miklos Vajna --- builtin-show-branch.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/builtin-show-branch.c b/builtin-show-branch.c index 306b850..828e6f8 100644 --- a/builtin-show-branch.c +++ b/builtin-show-branch.c @@ -365,8 +365,7 @@ static int append_ref(const char *refname, const unsigned char *sha1, return 0; } if (MAX_REVS <= ref_name_cnt) { - fprintf(stderr, "warning: ignoring %s; " - "cannot handle more than %d refs\n", + warning("ignoring %s; cannot handle more than %d refs", refname, MAX_REVS); return 0; } -- 1.6.2 ``` ## Miklos Vajna, 2009-03-24 01:09 Subject: [PATCH 8/9] builtin-show-ref: use warning() instead of fprintf(stderr, "warning: ") Message-ID: <607f1ab07464931305b62599805f898e2bf2ca98.1237856682.git.vmiklos@frugalware.org> URL: https://gitlist.dev/e/607f1ab07464931305b62599805f898e2bf2ca98.1237856682.git.vmiklos%40frugalware.org In-Reply-To: <5e3412be5d72bce2bf36fdcc2734b89386fdfc21.1237856682.git.vmiklos@frugalware.org> ``` Signed-off-by: Miklos Vajna --- builtin-show-ref.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/builtin-show-ref.c b/builtin-show-ref.c index 572b114..dc76c50 100644 --- a/builtin-show-ref.c +++ b/builtin-show-ref.c @@ -140,7 +140,7 @@ static int exclude_existing(const char *match) continue; } if (check_ref_format(ref)) { - fprintf(stderr, "warning: ref '%s' ignored\n", ref); + warning("ref '%s' ignored", ref); continue; } if (!string_list_has_string(&existing_refs, ref)) { -- 1.6.2 ``` ## Miklos Vajna, 2009-03-24 01:09 Subject: [PATCH 9/9] refs: use warning() instead of fprintf(stderr, "warning: ") Message-ID: URL: https://gitlist.dev/e/f6b2ede9af2bd40ce89f0251b32861098af46aad.1237856682.git.vmiklos%40frugalware.org In-Reply-To: <607f1ab07464931305b62599805f898e2bf2ca98.1237856682.git.vmiklos@frugalware.org> ``` Signed-off-by: Miklos Vajna --- refs.c | 8 +++----- 1 files changed, 3 insertions(+), 5 deletions(-) diff --git a/refs.c b/refs.c index 8d3c502..aeef257 100644 --- a/refs.c +++ b/refs.c @@ -996,7 +996,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt) err = unlink(git_path("logs/%s", lock->ref_name)); if (err && errno != ENOENT) - fprintf(stderr, "warning: unlink(%s) failed: %s", + warning("unlink(%s) failed: %s", git_path("logs/%s", lock->ref_name), strerror(errno)); invalidate_cached_refs(); unlock_ref(lock); @@ -1438,8 +1438,7 @@ int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char * if (get_sha1_hex(rec + 41, sha1)) die("Log %s is corrupt.", logfile); if (hashcmp(logged_sha1, sha1)) { - fprintf(stderr, - "warning: Log %s has gap after %s.\n", + warning("Log %s has gap after %s.", logfile, show_date(date, tz, DATE_RFC2822)); } } @@ -1451,8 +1450,7 @@ int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char * if (get_sha1_hex(rec + 41, logged_sha1)) die("Log %s is corrupt.", logfile); if (hashcmp(logged_sha1, sha1)) { - fprintf(stderr, - "warning: Log %s unexpectedly ended on %s.\n", + warning("Log %s unexpectedly ended on %s.", logfile, show_date(date, tz, DATE_RFC2822)); } } -- 1.6.2 ``` ## Erik Faye-Lund, 2009-03-24 01:17 Subject: Re: [PATCH 9/9] refs: use warning() instead of fprintf(stderr, "warning: ") Message-ID: <40aa078e0903231817g41fd1bc1s296121d85923f7ea@mail.gmail.com> URL: https://gitlist.dev/e/40aa078e0903231817g41fd1bc1s296121d85923f7ea%40mail.gmail.com In-Reply-To: ``` > @@ -996,7 +996,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt) > > err = unlink(git_path("logs/%s", lock->ref_name)); > if (err && errno != ENOENT) > - fprintf(stderr, "warning: unlink(%s) failed: %s", > + warning("unlink(%s) failed: %s", Doesn't this add a newline? The original string doesn't have a '\n' at the end of the format-field, but AFAIK warning() terminates with one. I'm not saying it's a problem, I'm just pointing it out. -- Erik "kusma" Faye-Lund kusmabite@gmail.com (+47) 986 59 656 ``` ## Miklos Vajna, 2009-03-24 01:23 Subject: Re: [PATCH 9/9] refs: use warning() instead of fprintf(stderr, "warning: ") Message-ID: <20090324012334.GL27459@genesis.frugalware.org> URL: https://gitlist.dev/e/20090324012334.GL27459%40genesis.frugalware.org In-Reply-To: <40aa078e0903231817g41fd1bc1s296121d85923f7ea@mail.gmail.com> ``` On Tue, Mar 24, 2009 at 02:17:24AM +0100, Erik Faye-Lund wrote: > > @@ -996,7 +996,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt) > > > > err = unlink(git_path("logs/%s", lock->ref_name)); > > if (err && errno != ENOENT) > > - fprintf(stderr, "warning: unlink(%s) failed: %s", > > + warning("unlink(%s) failed: %s", > > Doesn't this add a newline? The original string doesn't have a '\n' at > the end of the format-field, but AFAIK warning() terminates with one. > I'm not saying it's a problem, I'm just pointing it out. Yes, it adds. As Jeff pointed out earlier in this thread, the lack of newline was a bug: http://article.gmane.org/gmane.comp.version-control.git/110783 ``` ## Erik Faye-Lund, 2009-03-24 01:25 Subject: Re: [PATCH 2/9] builtin-apply: use warning() instead of fprintf(stderr, "warning: ") Message-ID: <40aa078e0903231825w54c9dca2xc68a849d245916a4@mail.gmail.com> URL: https://gitlist.dev/e/40aa078e0903231825w54c9dca2xc68a849d245916a4%40mail.gmail.com In-Reply-To: <9287282d966904f06ae4b3d7cfdf1d79eb645b69.1237856682.git.vmiklos@frugalware.org> ``` On Tue, Mar 24, 2009 at 2:09 AM, Miklos Vajna wrote: > @@ -2932,8 +2932,7 @@ static int write_out_one_reject(struct patch *patch) >        cnt = strlen(patch->new_name); >        if (ARRAY_SIZE(namebuf) <= cnt + 5) { >                cnt = ARRAY_SIZE(namebuf) - 5; > -               fprintf(stderr, > -                       "warning: truncating .rej filename to %.*s.rej", > +               warning("truncating .rej filename to %.*s.rej", Again, doesn't this change the output? -- Erik "kusma" Faye-Lund kusmabite@gmail.com (+47) 986 59 656 ``` ## Erik Faye-Lund, 2009-03-24 01:28 Subject: Re: [PATCH 9/9] refs: use warning() instead of fprintf(stderr, "warning: ") Message-ID: <40aa078e0903231828t2c5ff976m491ba0f739b0ea27@mail.gmail.com> URL: https://gitlist.dev/e/40aa078e0903231828t2c5ff976m491ba0f739b0ea27%40mail.gmail.com In-Reply-To: <20090324012334.GL27459@genesis.frugalware.org> ``` OK, sorry for not reading the old discussion before commenting. :) On Tue, Mar 24, 2009 at 2:23 AM, Miklos Vajna wrote: > On Tue, Mar 24, 2009 at 02:17:24AM +0100, Erik Faye-Lund wrote: >> > @@ -996,7 +996,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt) >> > >> >        err = unlink(git_path("logs/%s", lock->ref_name)); >> >        if (err && errno != ENOENT) >> > -               fprintf(stderr, "warning: unlink(%s) failed: %s", >> > +               warning("unlink(%s) failed: %s", >> >> Doesn't this add a newline? The original string doesn't have a '\n' at >> the end of the format-field, but AFAIK warning() terminates with one. >> I'm not saying it's a problem, I'm just pointing it out. > > Yes, it adds. As Jeff pointed out earlier in this thread, the lack of > newline was a bug: > > http://article.gmane.org/gmane.comp.version-control.git/110783 > -- Erik "kusma" Faye-Lund kusmabite@gmail.com (+47) 986 59 656 ``` ## Jeff King, 2009-03-24 07:35 Subject: Re: [PATCH 0/9] fprintf(stderr, "warning: ") -> warning() patches Message-ID: <20090324073513.GA32400@coredump.intra.peff.net> URL: https://gitlist.dev/e/20090324073513.GA32400%40coredump.intra.peff.net In-Reply-To: ``` On Tue, Mar 24, 2009 at 02:09:08AM +0100, Miklos Vajna wrote: > Okay, here it is, applies on top of master. All, except the http-push > one, applies cleanly on top of next as well here. I manually scanned the patches and they all look sane to me. Thanks. -Peff ``` ## Junio C Hamano, 2009-03-24 08:01 Subject: Re: [PATCH 0/9] fprintf(stderr, "warning: ") -> warning() patches Message-ID: <7vr60n2vtu.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vr60n2vtu.fsf%40gitster.siamese.dyndns.org In-Reply-To: <20090324073513.GA32400@coredump.intra.peff.net> ``` Jeff King writes: > On Tue, Mar 24, 2009 at 02:09:08AM +0100, Miklos Vajna wrote: > >> Okay, here it is, applies on top of master. All, except the http-push >> one, applies cleanly on top of next as well here. > > I manually scanned the patches and they all look sane to me. > Thanks. Thanks; I've done the same and three sets of eyeballs give me a warm fuzzy feeling ;-) Applied. ```