threads / patch / 18499

patch, 9 partsbuiltin-apply: use warning() instead of fprintf(stderr, "warning: ")

Subject: [PATCH 2/9] builtin-apply: use warning() instead of fprintf(stderr, "warning: ")

## tl;dr

16 messages between Mar 24, 2009 and Mar 24, 2009. Diffs are folded; open one to read it.

replies: 15people: 4as markdown or json

Miklos Vajna· Mar 24, 2009, 01:09 UTC · lore

[PATCH 0/9] fprintf(stderr, "warning: ") -> warning() patches

On Sun, Mar 22, 2009 at 02:58:46PM -0700, Junio C Hamano <gitster@pobox.com> 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· Mar 24, 2009, 01:09 UTC · re: Miklos Vajna · lore

[PATCH 1/9] http-push: using error() and warning() as appropriate

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 <vmiklos@frugalware.org>
---
 http-push.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
Show changes to http-push.c +3 −3
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· Mar 24, 2009, 01:09 UTC · re: Miklos Vajna · lore
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 builtin-apply.c |   15 +++++++--------
 1 files changed, 7 insertions(+), 8 deletions(-)
Show changes to builtin-apply.c +7 −8
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· Mar 24, 2009, 01:09 UTC · re: Miklos Vajna · lore

[PATCH 3/9] builtin-checkout: use warning() instead of fprintf(stderr, "warning: ")

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 builtin-checkout.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Show changes to builtin-checkout.c +2 −2
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· Mar 24, 2009, 01:09 UTC · re: Miklos Vajna · lore

[PATCH 4/9] builtin-fetch-pack: use warning() instead of fprintf(stderr, "warning: ")

---
 builtin-fetch-pack.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to builtin-fetch-pack.c +1 −1
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· Mar 24, 2009, 01:09 UTC · re: Miklos Vajna · lore

[PATCH 5/9] builtin-init-db: use warning() instead of fprintf(stderr, "warning: ")

---
 builtin-init-db.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)
Show changes to builtin-init-db.c +3 −4
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· Mar 24, 2009, 01:09 UTC · re: Miklos Vajna · lore

[PATCH 6/9] builtin-rm: use warning() instead of fprintf(stderr, "warning: ")

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 builtin-rm.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
Show changes to builtin-rm.c +1 −2
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· Mar 24, 2009, 01:09 UTC · re: Miklos Vajna · lore

[PATCH 7/9] builtin-show-branch: use warning() instead of fprintf(stderr, "warning: ")

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 builtin-show-branch.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
Show changes to builtin-show-branch.c +1 −2
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· Mar 24, 2009, 01:09 UTC · re: Miklos Vajna · lore

[PATCH 8/9] builtin-show-ref: use warning() instead of fprintf(stderr, "warning: ")

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 builtin-show-ref.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to builtin-show-ref.c +1 −1
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· Mar 24, 2009, 01:09 UTC · re: Miklos Vajna · lore

[PATCH 9/9] refs: use warning() instead of fprintf(stderr, "warning: ")

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 refs.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)
Show changes to refs.c +3 −5
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· Mar 24, 2009, 01:17 UTC · re: Miklos Vajna · lore

Re: [PATCH 9/9] refs: use warning() instead of fprintf(stderr, "warning: ")

Show 6 quoted lines
> @@ -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· Mar 24, 2009, 01:23 UTC · re: Erik Faye-Lund · lore

Re: [PATCH 9/9] refs: use warning() instead of fprintf(stderr, "warning: ")

On Tue, Mar 24, 2009 at 02:17:24AM +0100, Erik Faye-Lund <kusmabite@googlemail.com> wrote:
Show 10 quoted lines
> > @@ -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· Mar 24, 2009, 01:28 UTC · re: Miklos Vajna · lore

Re: [PATCH 9/9] refs: use warning() instead of fprintf(stderr, "warning: ")

OK, sorry for not reading the old discussion before commenting. :)
On Tue, Mar 24, 2009 at 2:23 AM, Miklos Vajna <vmiklos@frugalware.org> wrote:
Show 17 quoted lines
> On Tue, Mar 24, 2009 at 02:17:24AM +0100, Erik Faye-Lund <kusmabite@googlemail.com> 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
Erik Faye-Lund· Mar 24, 2009, 01:25 UTC · re: Miklos Vajna · lore

Re: [PATCH 2/9] builtin-apply: use warning() instead of fprintf(stderr, "warning: ")

On Tue, Mar 24, 2009 at 2:09 AM, Miklos Vajna <vmiklos@frugalware.org> wrote:
Show 7 quoted lines
> @@ -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
Jeff King· Mar 24, 2009, 07:35 UTC · re: Miklos Vajna · lore

Re: [PATCH 0/9] fprintf(stderr, "warning: ") -> warning() patches

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· Mar 24, 2009, 08:01 UTC · re: Jeff King · lore

Re: [PATCH 0/9] fprintf(stderr, "warning: ") -> warning() patches

Jeff King <peff@peff.net> writes:
Show 7 quoted lines
> 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.

← back to recent threads