Add warning base class and use that for warnings

5 files changed, 125 insertions, 32 deletions

diff --git a/include/git2/sys/warning.h b/include/git2/sys/warning.h
index 1b833ef4b..ec1262a6b 100644
--- a/include/git2/sys/warning.h
+++ b/include/git2/sys/warning.h
@@ -11,27 +11,41 @@ GIT_BEGIN_DECL
 
 typedef enum {
 	GIT_WARNING_NONE = 0,
-	GIT_WARNING_INVALID_SIGNATURE_TIMESTAMP,
-	GIT_WARNING_INVALID_SIGNATURE_TIMEZONE,
+	GIT_WARNING_INVALID_DATA__SIGNATURE_TIMESTAMP,
+	GIT_WARNING_INVALID_DATA__SIGNATURE_TIMEZONE,
 } git_warning_t;
 
 /**
+ * Base class for warnings
+ */
+typedef struct git_warning git_warning;
+struct git_warning {
+	git_warning_t type;
+	const char *message;
+};
+
+/**
+ * Subclass of warning for invalid data string
+ */
+typedef struct {
+	git_warning base;
+	const char *invalid_data;
+	int invalid_data_len;
+} git_warning_invalid_data;
+
+/**
  * Type for warning callbacks.
  *
  * Using `git_warning_set_callback(cb, payload)` you can set a warning
  * callback function (and payload) that will be used to issue various
  * warnings when recoverable data problems are encountered inside libgit2.
- * It will be passed several parameters describing the problem.
+ * It will be passed a warning structure describing the problem.
  *
- * @param warning A git_warning_t value for the specific situation
- * @param message A message explaining the details of the warning
+ * @param warning A git_warning structure for the specific situation
  * @param payload The payload set when callback function was specified
  * @return 0 to continue, <0 to convert the warning to an error
  */
-typedef int (*git_warning_callback)(
-	git_warning_t warning,
-	const char *message,
-	void *payload);
+typedef int (*git_warning_callback)(git_warning *warning, void *payload);
 
 /**
  * Set the callback to be invoked when an invalid but recoverable
diff --git a/src/signature.c b/src/signature.c
index dbaa9bd17..a513b0e65 100644
--- a/src/signature.c
+++ b/src/signature.c
@@ -199,10 +199,10 @@ int git_signature__parse(
 			time_end = git__skip_over_to_space(time_start);
 
 			/* warn (and return error if requested) */
-			if (git_warning(
-					GIT_WARNING_INVALID_SIGNATURE_TIMESTAMP,
-					"invalid signature timestamp '%.*s'",
-					(int)(time_end - time_start), time_start) < 0)
+			if (git_warn_invalid_data(
+					GIT_WARNING_INVALID_DATA__SIGNATURE_TIMESTAMP,
+					time_start, (int)(time_end - time_start),
+					"invalid signature %stimestamp", header) < 0)
 				return signature_error("invalid Unix timestamp");
 		}
 
@@ -223,10 +223,10 @@ int git_signature__parse(
 				tz_end = git__skip_over_to_space(tz_start);
 
 				/* warn (and return error if requested) */
-				if (git_warning(
-						GIT_WARNING_INVALID_SIGNATURE_TIMEZONE,
-						"invalid signature timezone '%.*s'",
-						(int)(tz_end - tz_start), tz_start) < 0)
+				if (git_warn_invalid_data(
+						GIT_WARNING_INVALID_DATA__SIGNATURE_TIMEZONE,
+						tz_start, (int)(tz_end - tz_start),
+						"invalid timezone in signature %s", header) < 0)
 					return signature_error("invalid timezone");
 			}
 
diff --git a/src/warning.c b/src/warning.c
index 52b71f272..1066c2c8d 100644
--- a/src/warning.c
+++ b/src/warning.c
@@ -18,28 +18,71 @@ void git_warning_set_callback(git_warning_callback cb, void *payload)
 	_warning_payload = payload;
 }
 
-int git_warning(
-	git_warning_t warning,
-	const char *fmt,
-	...)
+static int git_warning__send(
+	git_warning *warning, const char *fmt, va_list ap)
 {
 	int error = 0;
 	git_buf buf = GIT_BUF_INIT;
 	git_warning_callback cb = _warning_cb;
-	va_list arglist;
 
 	if (!cb)
 		return 0;
 
-	va_start(arglist, fmt);
-	error = git_buf_vprintf(&buf, fmt, arglist);
-	va_end(arglist);
-
-	if (!error)
-		error = cb(warning, git_buf_cstr(&buf), _warning_payload);
+	if (!(error = git_buf_vprintf(&buf, fmt, ap))) {
+		warning->message = git_buf_cstr(&buf);
+		error = cb(warning, _warning_payload);
+	}
 
 	git_buf_free(&buf);
 
 	return error;
 }
 
+int git_warn(
+	git_warning_t type,
+	const char *fmt,
+	...)
+{
+	int error;
+	va_list ap;
+	git_warning warning;
+
+	if (!_warning_cb)
+		return 0;
+
+	warning.type = type;
+
+	va_start(ap, fmt);
+	error = git_warning__send(&warning, fmt, ap);
+	va_end(ap);
+
+	return error;
+}
+
+int git_warn_invalid_data(
+	git_warning_t type,
+	const char *data,
+	int datalen,
+	const char *fmt,
+	...)
+{
+	int error;
+	va_list ap;
+	git_warning_invalid_data warning;
+
+	if (!_warning_cb)
+		return 0;
+
+	warning.base.type = type;
+	warning.invalid_data = git__strndup(data, datalen);
+	GITERR_CHECK_ALLOC(warning.invalid_data);
+	warning.invalid_data_len = datalen;
+
+	va_start(ap, fmt);
+	error = git_warning__send((git_warning *)&warning, fmt, ap);
+	va_end(ap);
+
+	git__free((char *)warning.invalid_data);
+
+	return error;
+}
diff --git a/src/warning.h b/src/warning.h
index dc9308c74..6d25783f4 100644
--- a/src/warning.h
+++ b/src/warning.h
@@ -10,9 +10,16 @@
 #include "common.h"
 #include "git2/sys/warning.h"
 
-extern int git_warning(
+int git_warn(
 	git_warning_t warning,
 	const char *fmt,
 	...);
 
+int git_warn_invalid_data(
+	git_warning_t warning,
+	const char *data,
+	int datalen,
+	const char *fmt,
+	...);
+
 #endif
diff --git a/tests/commit/parse.c b/tests/commit/parse.c
index 8bd261c03..07f304ce3 100644
--- a/tests/commit/parse.c
+++ b/tests/commit/parse.c
@@ -177,25 +177,54 @@ void test_commit_parse__signature(void)
 		assert_signature_doesnt_parse(failcase);
 }
 
-static int fail_on_warn(git_warning_t w, const char *m, void *p)
+static int pass_on_warn(git_warning *warning, void *payload)
 {
-	GIT_UNUSED(w); GIT_UNUSED(m); GIT_UNUSED(p);
+	git_warning *expected = payload;
+	cl_assert_equal_i(expected->type, warning->type);
+	if (expected->message)
+		cl_assert(strstr(warning->message, expected->message) != NULL);
+	return 0;
+}
+
+static int fail_on_warn(git_warning *warning, void *payload)
+{
+	git_warning *expected = payload;
+	cl_assert_equal_i(expected->type, warning->type);
+	if (expected->message)
+		cl_assert(strstr(warning->message, expected->message) != NULL);
 	return -1;
 }
 
 void test_commit_parse__signature_semivalid(void)
 {
+	git_warning expected = { 0 };
 	passing_signature_test_case passcase = {"author Vicent Marti <tanoku@gmail.com> 9999999999998589934592 \n", "author ", "Vicent Marti", "tanoku@gmail.com", -1, 0};
 	failing_signature_test_case failcase1 = {"author Vicent Marti <tanoku@gmail.com> 9999999999998589934592 \n", "author "};
 	failing_signature_test_case failcase2 = {"author Vicent Marti <tanoku@gmail.com> 998589934592 +123412341234123412341234 \n", "author "};
+	failing_signature_test_case failcase3 = {"committer Vicent Marti <tanoku@gmail.com> 998589934592 +123412341234123412341234 \n", "committer "};
+
+	expected.type = GIT_WARNING_INVALID_DATA__SIGNATURE_TIMESTAMP;
+	expected.message = "author";
+	git_warning_set_callback(pass_on_warn, &expected);
 
 	assert_signature_parses(&passcase);
 
-	git_warning_set_callback(fail_on_warn, NULL);
+	expected.type = GIT_WARNING_INVALID_DATA__SIGNATURE_TIMESTAMP;
+	expected.message = "author";
+	git_warning_set_callback(fail_on_warn, &expected);
 
 	assert_signature_doesnt_parse(&failcase1);
+
+	expected.type = GIT_WARNING_INVALID_DATA__SIGNATURE_TIMEZONE;
+	expected.message = "author";
+
 	assert_signature_doesnt_parse(&failcase2);
 
+	expected.type = GIT_WARNING_INVALID_DATA__SIGNATURE_TIMEZONE;
+	expected.message = "committer";
+
+	assert_signature_doesnt_parse(&failcase3);
+
 	git_warning_set_callback(NULL, NULL);
 }