From bde336ea51df071de922827ae322df7b01b059ce Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Thu, 29 Nov 2012 12:26:09 -0800 Subject: [PATCH 01/18] Add version fields and init macros for public input structs. --- include/git2/checkout.h | 3 ++- include/git2/config.h | 3 +++ include/git2/diff.h | 21 +++++++++++++-------- include/git2/odb_backend.h | 4 ++++ include/git2/remote.h | 4 ++++ include/git2/repository.h | 4 ++++ include/git2/status.h | 3 +++ include/git2/transport.h | 9 +++++++++ include/git2/types.h | 4 ++++ tests-clar/commit/parse.c | 4 ++-- 10 files changed, 48 insertions(+), 11 deletions(-) diff --git a/include/git2/checkout.h b/include/git2/checkout.h index bd988db2c..ea6e219ba 100644 --- a/include/git2/checkout.h +++ b/include/git2/checkout.h @@ -186,7 +186,8 @@ typedef struct git_checkout_opts { git_strarray paths; } git_checkout_opts; -#define GIT_CHECKOUT_OPTS_INIT {1, 0} +#define GIT_CHECKOUT_OPTS_VERSION 1 +#define GIT_CHECKOUT_OPTS_INIT {GIT_CHECKOUT_OPTS_VERSION, 0} /** * Updates files in the index and the working tree to match the content of the diff --git a/include/git2/config.h b/include/git2/config.h index af4d54044..4855babec 100644 --- a/include/git2/config.h +++ b/include/git2/config.h @@ -49,6 +49,7 @@ typedef int (*git_config_foreach_cb)(const git_config_entry *, void *); * access a configuration file */ struct git_config_backend { + unsigned int version; struct git_config *cfg; /* Open means open the file/database and parse if necessary */ @@ -62,6 +63,8 @@ struct git_config_backend { int (*refresh)(struct git_config_backend *); void (*free)(struct git_config_backend *); }; +#define GIT_CONFIG_BACKEND_VERSION 1 +#define GIT_CONFIG_BACKEND_INIT {GIT_CONFIG_BACKEND_VERSION, 0} typedef enum { GIT_CVAR_FALSE = 0, diff --git a/include/git2/diff.h b/include/git2/diff.h index fd00378af..855e4beed 100644 --- a/include/git2/diff.h +++ b/include/git2/diff.h @@ -106,16 +106,19 @@ typedef enum { * - max_size: maximum blob size to diff, above this treated as binary */ typedef struct { - unsigned int version; /**< version for the struct */ - uint32_t flags; /**< defaults to GIT_DIFF_NORMAL */ - uint16_t context_lines; /**< defaults to 3 */ - uint16_t interhunk_lines; /**< defaults to 0 */ - char *old_prefix; /**< defaults to "a" */ - char *new_prefix; /**< defaults to "b" */ - git_strarray pathspec; /**< defaults to show all paths */ - git_off_t max_size; /**< defaults to 512Mb */ + unsigned int version; /**< version for the struct */ + uint32_t flags; /**< defaults to git_diff_normal */ + uint16_t context_lines; /**< defaults to 3 */ + uint16_t interhunk_lines; /**< defaults to 0 */ + char *old_prefix; /**< defaults to "a" */ + char *new_prefix; /**< defaults to "b" */ + git_strarray pathspec; /**< defaults to show all paths */ + git_off_t max_size; /**< defaults to 512mb */ } git_diff_options; +#define GIT_DIFF_OPTIONS_VERSION 1 +#define GIT_DIFF_OPTIONS_INIT = {GIT_DIFF_OPTIONS_VERSION, 0} + /** * The diff list object that contains all individual file deltas. */ @@ -304,6 +307,8 @@ typedef struct { unsigned int target_limit; } git_diff_find_options; +#define GIT_DIFF_FIND_OPTIONS_VERSION 1 +#define GIT_DIFF_FIND_OPTIONS_INIT {GIT_DIFF_FIND_OPTIONS_VERSION, 0} /** @name Diff List Generator Functions * diff --git a/include/git2/odb_backend.h b/include/git2/odb_backend.h index 04658f9b3..3963ae531 100644 --- a/include/git2/odb_backend.h +++ b/include/git2/odb_backend.h @@ -33,6 +33,7 @@ typedef int (*git_odb_foreach_cb)(const git_oid *id, void *payload); * An instance for a custom backend */ struct git_odb_backend { + unsigned int version; git_odb *odb; /* read and read_prefix each return to libgit2 a buffer which @@ -98,6 +99,9 @@ struct git_odb_backend { void (* free)(struct git_odb_backend *); }; +#define GIT_ODB_BACKEND_VERSION 1 +#define GIT_ODB_BACKEND_INIT {GIT_ODB_BACKEND_VERSION, 0} + /** Streaming mode */ enum { GIT_STREAM_RDONLY = (1 << 1), diff --git a/include/git2/remote.h b/include/git2/remote.h index 6c70d7fbc..e5b60b951 100644 --- a/include/git2/remote.h +++ b/include/git2/remote.h @@ -338,12 +338,16 @@ typedef enum git_remote_completion_type { * Set the calbacks to be called by the remote. */ struct git_remote_callbacks { + unsigned int version; void (*progress)(const char *str, int len, void *data); int (*completion)(git_remote_completion_type type, void *data); int (*update_tips)(const char *refname, const git_oid *a, const git_oid *b, void *data); void *payload; }; +#define GIT_REMOTE_CALLBACKS_VERSION 1 +#define GIT_REMOTE_CALLBACKS_INIT {GIT_REMOTE_CALLBACKS_VERSION, 0} + /** * Set the callbacks for a remote * diff --git a/include/git2/repository.h b/include/git2/repository.h index e91108a33..0d82b9810 100644 --- a/include/git2/repository.h +++ b/include/git2/repository.h @@ -239,6 +239,7 @@ typedef enum { * will be added pointing to this URL. */ typedef struct { + unsigned int version; uint32_t flags; uint32_t mode; const char *workdir_path; @@ -248,6 +249,9 @@ typedef struct { const char *origin_url; } git_repository_init_options; +#define GIT_REPOSITORY_INIT_OPTIONS_VERSION 1 +#define GIT_REPOSITORY_INIT_OPTIONS_INIT {GIT_REPOSITORY_INIT_OPTIONS_VERSION, 0} + /** * Create a new Git repository in the given folder with extended controls. * diff --git a/include/git2/status.h b/include/git2/status.h index c6926f343..1211f1d9f 100644 --- a/include/git2/status.h +++ b/include/git2/status.h @@ -164,6 +164,9 @@ typedef struct { git_strarray pathspec; } git_status_options; +#define GIT_STATUS_OPTIONS_VERSION 1 +#define GIT_STATUS_OPTIONS_INIT {GIT_STATUS_OPTIONS_VERSION, 0} + /** * Gather file status information and run callbacks as requested. * diff --git a/include/git2/transport.h b/include/git2/transport.h index 61726922f..84d71c612 100644 --- a/include/git2/transport.h +++ b/include/git2/transport.h @@ -31,11 +31,15 @@ typedef enum { /* The base structure for all credential types */ typedef struct git_cred { + unsigned int version; /* This should update if subtypes are extended */ git_credtype_t credtype; void (*free)( struct git_cred *cred); } git_cred; +#define GIT_CRED_VERSION 1 +#define GIT_CRED_INIT {GIT_CRED_VERSION, 0} + /* A plaintext username and password */ typedef struct git_cred_userpass_plaintext { git_cred parent; @@ -82,6 +86,7 @@ typedef enum { typedef void (*git_transport_message_cb)(const char *str, int len, void *data); typedef struct git_transport { + unsigned int version; /* Set progress and error callbacks */ int (*set_callbacks)(struct git_transport *transport, git_transport_message_cb progress_cb, @@ -140,6 +145,9 @@ typedef struct git_transport { void (*free)(struct git_transport *transport); } git_transport; +#define GIT_TRANSPORT_VERSION 1 +#define GIT_TRANSPORT_INIT {GIT_TRANSPORT_VERSION, 0} + /** * Function to use to create a transport from a URL. The transport database * is scanned to find a transport that implements the scheme of the URI (i.e. @@ -284,6 +292,7 @@ typedef int (*git_smart_subtransport_cb)( typedef struct git_smart_subtransport_definition { /* The function to use to create the git_smart_subtransport */ git_smart_subtransport_cb callback; + /* True if the protocol is stateless; false otherwise. For example, * http:// is stateless, but git:// is not. */ unsigned rpc : 1; diff --git a/include/git2/types.h b/include/git2/types.h index 06fcf3613..d83b1d14b 100644 --- a/include/git2/types.h +++ b/include/git2/types.h @@ -151,11 +151,15 @@ typedef struct git_time { /** An action signature (e.g. for committers, taggers, etc) */ typedef struct git_signature { + unsigned int version; char *name; /** full name of the author */ char *email; /** email of the author */ git_time when; /** time when the action happened */ } git_signature; +#define GIT_SIGNATURE_VERSION 1 +#define GIT_SIGNATURE_INIT {GIT_SIGNATURE_VERSION, 0} + /** In-memory representation of a reference. */ typedef struct git_reference git_reference; diff --git a/tests-clar/commit/parse.c b/tests-clar/commit/parse.c index bbb502cb5..37e38db53 100644 --- a/tests-clar/commit/parse.c +++ b/tests-clar/commit/parse.c @@ -149,7 +149,7 @@ void test_commit_parse__signature(void) { const char *str = passcase->string; size_t len = strlen(passcase->string); - struct git_signature person = {NULL, NULL, {0, 0}}; + struct git_signature person = GIT_SIGNATURE_INIT; cl_git_pass(git_signature__parse(&person, &str, str + len, passcase->header, '\n')); cl_assert(strcmp(passcase->name, person.name) == 0); cl_assert(strcmp(passcase->email, person.email) == 0); @@ -162,7 +162,7 @@ void test_commit_parse__signature(void) { const char *str = failcase->string; size_t len = strlen(failcase->string); - git_signature person = {NULL, NULL, {0, 0}}; + git_signature person = GIT_SIGNATURE_INIT; cl_git_fail(git_signature__parse(&person, &str, str + len, failcase->header, '\n')); git__free(person.name); git__free(person.email); } From f4fc9fdba03dd4975229243d7c1debd00c9d1f18 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Thu, 29 Nov 2012 12:26:40 -0800 Subject: [PATCH 02/18] Cleanup nitpicky things --- include/git2/refs.h | 2 +- include/git2/strarray.h | 5 ++--- tests-clar/network/fetch.c | 1 + tests-clar/refs/branches/tracking.c | 3 ++- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/include/git2/refs.h b/include/git2/refs.h index c92646115..af6c2f43b 100644 --- a/include/git2/refs.h +++ b/include/git2/refs.h @@ -381,7 +381,7 @@ GIT_EXTERN(int) git_reference_foreach_glob( git_repository *repo, const char *glob, unsigned int list_flags, - int (*callback)(const char *reference_name, void *payload), + git_reference_foreach_cb callback, void *payload); /** diff --git a/include/git2/strarray.h b/include/git2/strarray.h index 030567978..338d13873 100644 --- a/include/git2/strarray.h +++ b/include/git2/strarray.h @@ -19,11 +19,10 @@ GIT_BEGIN_DECL /** Array of strings */ -typedef struct _git_strarray git_strarray; -struct _git_strarray { +typedef struct git_strarray { char **strings; size_t count; -}; +} git_strarray; /** * Close a string array object diff --git a/tests-clar/network/fetch.c b/tests-clar/network/fetch.c index 84c947291..26937c608 100644 --- a/tests-clar/network/fetch.c +++ b/tests-clar/network/fetch.c @@ -78,6 +78,7 @@ void test_network_fetch__no_tags_http(void) static void transferProgressCallback(const git_transfer_progress *stats, void *payload) { + GIT_UNUSED(stats); bool *invoked = (bool *)payload; *invoked = true; } diff --git a/tests-clar/refs/branches/tracking.c b/tests-clar/refs/branches/tracking.c index e8b2f24d7..84d2961ae 100644 --- a/tests-clar/refs/branches/tracking.c +++ b/tests-clar/refs/branches/tracking.c @@ -65,7 +65,8 @@ static void assert_merge_and_or_remote_key_missing(git_repository *repository, g { git_reference *branch; - cl_git_pass(git_branch_create(&branch, repository, entry_name, target, 0)); + cl_assert_equal_i(GIT_OBJ_COMMIT, git_object_type(target)); + cl_git_pass(git_branch_create(&branch, repository, entry_name, (git_commit*)target, 0)); cl_assert_equal_i(GIT_ENOTFOUND, git_branch_tracking(&tracking, branch)); From b81aa2f1deb98eb7f9e60ac96696e69a9a49d8f8 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Thu, 29 Nov 2012 14:06:40 -0800 Subject: [PATCH 03/18] Deploy GIT_CHECKOUT_OPTS_INIT --- examples/network/clone.c | 2 +- src/checkout.c | 22 +++++++++++++++++++++- src/reset.c | 3 +-- src/stash.c | 4 +--- tests-clar/checkout/checkout_util.h | 5 +++++ tests-clar/checkout/index.c | 23 +++++++++++++++++++++-- tests-clar/checkout/tree.c | 3 ++- tests-clar/checkout/typechange.c | 2 +- 8 files changed, 53 insertions(+), 11 deletions(-) create mode 100644 tests-clar/checkout/checkout_util.h diff --git a/examples/network/clone.c b/examples/network/clone.c index a718f3084..7596523c2 100644 --- a/examples/network/clone.c +++ b/examples/network/clone.c @@ -51,7 +51,7 @@ int do_clone(git_repository *repo, int argc, char **argv) { progress_data pd; git_repository *cloned_repo = NULL; - git_checkout_opts checkout_opts; + git_checkout_opts checkout_opts = GIT_CHECKOUT_OPTS_INIT; const char *url = argv[1]; const char *path = argv[2]; int error; diff --git a/src/checkout.c b/src/checkout.c index a3166bfa5..2a7ad70be 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -225,10 +225,12 @@ static int retrieve_symlink_caps(git_repository *repo, bool *can_symlink) static void normalize_options( git_checkout_opts *normalized, git_checkout_opts *proposed) { + git_checkout_opts init_opts = GIT_CHECKOUT_OPTS_INIT; + assert(normalized); if (!proposed) - memset(normalized, 0, sizeof(git_checkout_opts)); + memmove(normalized, &init_opts, sizeof(git_checkout_opts)); else memmove(normalized, proposed, sizeof(git_checkout_opts)); @@ -601,6 +603,19 @@ static int checkout_create_submodules( return 0; } +static bool opts_is_valid_version(git_checkout_opts *opts) +{ + if (!opts) + return true; + + if (opts->version > 0 && opts->version <= GIT_CHECKOUT_OPTS_VERSION) + return true; + + giterr_set(GITERR_INVALID, "Invalid version %d on git_checkout_opts structure", + opts->version); + return false; +} + int git_checkout_index( git_repository *repo, git_index *index, @@ -624,6 +639,11 @@ int git_checkout_index( GIT_DIFF_INCLUDE_UNMODIFIED | GIT_DIFF_INCLUDE_UNTRACKED | GIT_DIFF_INCLUDE_TYPECHANGE | GIT_DIFF_SKIP_BINARY_CHECK; + if (!opts_is_valid_version(opts)) { + error = -1; + goto cleanup; + } + if (opts && opts->paths.count > 0) diff_opts.pathspec = opts->paths; diff --git a/src/reset.c b/src/reset.c index d410a8806..17b4b900c 100644 --- a/src/reset.c +++ b/src/reset.c @@ -69,7 +69,7 @@ int git_reset( git_index *index = NULL; git_tree *tree = NULL; int error = -1; - git_checkout_opts opts; + git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT; assert(repo && target); assert(reset_type == GIT_RESET_SOFT @@ -136,7 +136,6 @@ int git_reset( goto cleanup; } - memset(&opts, 0, sizeof(opts)); opts.checkout_strategy = GIT_CHECKOUT_FORCE; if (git_checkout_index(repo, NULL, &opts) < 0) { diff --git a/src/stash.c b/src/stash.c index 107cbe3ca..edd8c55db 100644 --- a/src/stash.c +++ b/src/stash.c @@ -498,9 +498,7 @@ static int reset_index_and_workdir( git_commit *commit, bool remove_untracked) { - git_checkout_opts opts; - - memset(&opts, 0, sizeof(git_checkout_opts)); + git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT; opts.checkout_strategy = GIT_CHECKOUT_UPDATE_MODIFIED | GIT_CHECKOUT_UPDATE_UNTRACKED; diff --git a/tests-clar/checkout/checkout_util.h b/tests-clar/checkout/checkout_util.h new file mode 100644 index 000000000..d6aa7ed47 --- /dev/null +++ b/tests-clar/checkout/checkout_util.h @@ -0,0 +1,5 @@ +GIT_INLINE(void) reset_checkout_opts(git_checkout_opts *opts) +{ + git_checkout_opts init_opts = GIT_CHECKOUT_OPTS_INIT; + memmove(opts, &init_opts, sizeof(git_checkout_opts)); +} diff --git a/tests-clar/checkout/index.c b/tests-clar/checkout/index.c index b6d637223..e86dfe954 100644 --- a/tests-clar/checkout/index.c +++ b/tests-clar/checkout/index.c @@ -2,6 +2,7 @@ #include "git2/checkout.h" #include "repository.h" +#include "checkout_util.h" static git_repository *g_repo; static git_checkout_opts g_opts; @@ -25,7 +26,7 @@ void test_checkout_index__initialize(void) { git_tree *tree; - memset(&g_opts, 0, sizeof(g_opts)); + reset_checkout_opts(&g_opts); g_opts.checkout_strategy = GIT_CHECKOUT_SAFE; g_repo = cl_git_sandbox_init("testrepo"); @@ -66,7 +67,7 @@ void test_checkout_index__cannot_checkout_a_bare_repository(void) { test_checkout_index__cleanup(); - memset(&g_opts, 0, sizeof(g_opts)); + reset_checkout_opts(&g_opts); g_repo = cl_git_sandbox_init("testrepo.git"); cl_git_fail(git_checkout_index(g_repo, NULL, NULL)); @@ -426,3 +427,21 @@ void test_checkout_index__can_overcome_name_clashes(void) git_index_free(index); } + +void test_checkout_index__validates_struct_version(void) +{ + const git_error *err; + + g_opts.version = 1024; + cl_git_fail(git_checkout_index(g_repo, NULL, &g_opts)); + + err = giterr_last(); + cl_assert_equal_i(err->klass, GITERR_INVALID); + + g_opts.version = 0; + giterr_clear(); + cl_git_fail(git_checkout_index(g_repo, NULL, &g_opts)); + + err = giterr_last(); + cl_assert_equal_i(err->klass, GITERR_INVALID); +} diff --git a/tests-clar/checkout/tree.c b/tests-clar/checkout/tree.c index 534c46086..fe5bd4ff8 100644 --- a/tests-clar/checkout/tree.c +++ b/tests-clar/checkout/tree.c @@ -2,6 +2,7 @@ #include "git2/checkout.h" #include "repository.h" +#include "checkout_util.h" static git_repository *g_repo; static git_checkout_opts g_opts; @@ -11,7 +12,7 @@ void test_checkout_tree__initialize(void) { g_repo = cl_git_sandbox_init("testrepo"); - memset(&g_opts, 0, sizeof(g_opts)); + reset_checkout_opts(&g_opts); g_opts.checkout_strategy = GIT_CHECKOUT_SAFE; } diff --git a/tests-clar/checkout/typechange.c b/tests-clar/checkout/typechange.c index cd34885de..98c15bcb7 100644 --- a/tests-clar/checkout/typechange.c +++ b/tests-clar/checkout/typechange.c @@ -38,7 +38,7 @@ void test_checkout_typechange__checkout_typechanges(void) { int i; git_object *obj; - git_checkout_opts opts = {0}; + git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT; opts.checkout_strategy = GIT_CHECKOUT_FORCE; From 691776213947e59a3928aab09e97a64b65e990ab Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Thu, 29 Nov 2012 14:07:50 -0800 Subject: [PATCH 04/18] Deploy git_config_backend version --- src/config.c | 15 +++++++++++++++ src/config_file.c | 4 ++-- tests-clar/config/backend.c | 20 ++++++++++++++++++++ 3 files changed, 37 insertions(+), 2 deletions(-) create mode 100644 tests-clar/config/backend.c diff --git a/src/config.c b/src/config.c index 6347f7df7..913108abb 100644 --- a/src/config.c +++ b/src/config.c @@ -248,6 +248,18 @@ int git_config_open_level( return 0; } +static bool config_backend_has_valid_version(git_config_backend *backend) +{ + if (!backend) + return true; + + if (backend->version > 0 && backend->version <= GIT_CONFIG_BACKEND_VERSION) + return true; + + giterr_set(GITERR_INVALID, "Invalid version %d for git_config_backend", backend->version); + return false; +} + int git_config_add_backend( git_config *cfg, git_config_backend *file, @@ -259,6 +271,9 @@ int git_config_add_backend( assert(cfg && file); + if (!config_backend_has_valid_version(file)) + return -1; + if ((result = file->open(file, level)) < 0) return result; diff --git a/src/config_file.c b/src/config_file.c index 354a91986..6e29832d4 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -545,10 +545,10 @@ int git_config_file__ondisk(git_config_backend **out, const char *path) { diskfile_backend *backend; - backend = git__malloc(sizeof(diskfile_backend)); + backend = git__calloc(1, sizeof(diskfile_backend)); GITERR_CHECK_ALLOC(backend); - memset(backend, 0x0, sizeof(diskfile_backend)); + backend->parent.version = GIT_CONFIG_BACKEND_VERSION; backend->file_path = git__strdup(path); GITERR_CHECK_ALLOC(backend->file_path); diff --git a/tests-clar/config/backend.c b/tests-clar/config/backend.c new file mode 100644 index 000000000..1cf770263 --- /dev/null +++ b/tests-clar/config/backend.c @@ -0,0 +1,20 @@ +#include "clar_libgit2.h" + +void test_config_backend__checks_version(void) +{ + git_config *cfg; + git_config_backend backend = GIT_CONFIG_BACKEND_INIT; + backend.version = 1024; + const git_error *err; + + cl_git_pass(git_config_new(&cfg)); + cl_git_fail(git_config_add_backend(cfg, &backend, 0, false)); + err = giterr_last(); + cl_assert_equal_i(GITERR_INVALID, err->klass); + + giterr_clear(); + backend.version = 1024; + cl_git_fail(git_config_add_backend(cfg, &backend, 0, false)); + err = giterr_last(); + cl_assert_equal_i(GITERR_INVALID, err->klass); +} From 2f8d30becb4801d869188d2d46ca1512843e8698 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Thu, 29 Nov 2012 15:05:04 -0800 Subject: [PATCH 05/18] Deploy GIT_DIFF_OPTIONS_INIT --- examples/diff.c | 2 +- include/git2/diff.h | 2 +- src/checkout.c | 2 +- src/diff.c | 7 ++++--- src/diff.h | 14 +++++++++++++ src/diff_output.c | 3 +++ src/stash.c | 4 ++-- src/status.c | 3 +-- src/submodule.c | 3 +-- tests-clar/diff/blob.c | 24 ++++++++++++++++++++- tests-clar/diff/diff_helpers.h | 7 +++++++ tests-clar/diff/diffiter.c | 30 +++++++++++++++++++++++---- tests-clar/diff/index.c | 25 ++++++++++++++++++++-- tests-clar/diff/rename.c | 3 +-- tests-clar/diff/tree.c | 34 +++++++++++++++++++++++++++--- tests-clar/diff/workdir.c | 38 ++++++++++++++++++++++++++-------- 16 files changed, 168 insertions(+), 33 deletions(-) diff --git a/examples/diff.c b/examples/diff.c index a465182ba..b81a8682a 100644 --- a/examples/diff.c +++ b/examples/diff.c @@ -133,7 +133,7 @@ int main(int argc, char *argv[]) { git_repository *repo = NULL; git_tree *t1 = NULL, *t2 = NULL; - git_diff_options opts; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff; int i, color = -1, compact = 0, cached = 0; char *a, *dir = ".", *treeish1 = NULL, *treeish2 = NULL; diff --git a/include/git2/diff.h b/include/git2/diff.h index 855e4beed..16a24b5f6 100644 --- a/include/git2/diff.h +++ b/include/git2/diff.h @@ -117,7 +117,7 @@ typedef struct { } git_diff_options; #define GIT_DIFF_OPTIONS_VERSION 1 -#define GIT_DIFF_OPTIONS_INIT = {GIT_DIFF_OPTIONS_VERSION, 0} +#define GIT_DIFF_OPTIONS_INIT {GIT_DIFF_OPTIONS_VERSION, 0} /** * The diff list object that contains all individual file deltas. diff --git a/src/checkout.c b/src/checkout.c index 2a7ad70be..06407bb30 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -622,7 +622,7 @@ int git_checkout_index( git_checkout_opts *opts) { git_diff_list *diff = NULL; - git_diff_options diff_opts = {0}; + git_diff_options diff_opts = GIT_DIFF_OPTIONS_INIT; git_checkout_opts checkout_opts; checkout_diff_data data; git_buf workdir = GIT_BUF_INIT; diff --git a/src/diff.c b/src/diff.c index 86f76f9c0..722acbead 100644 --- a/src/diff.c +++ b/src/diff.c @@ -755,14 +755,15 @@ fail: return error; } - #define DIFF_FROM_ITERATORS(MAKE_FIRST, MAKE_SECOND) do { \ git_iterator *a = NULL, *b = NULL; \ char *pfx = opts ? git_pathspec_prefix(&opts->pathspec) : NULL; \ - if (!(error = MAKE_FIRST) && !(error = MAKE_SECOND)) \ + if (!git_diff__opts_has_valid_version(opts)) \ + error = -1; \ + else if (!(error = MAKE_FIRST) && !(error = MAKE_SECOND)) \ error = diff_from_iterators(diff, repo, a, b, opts); \ git__free(pfx); git_iterator_free(a); git_iterator_free(b); \ - } while (0) +} while (0) int git_diff_tree_to_tree( git_diff_list **diff, diff --git a/src/diff.h b/src/diff.h index 1e3be7593..1f45af1cd 100644 --- a/src/diff.h +++ b/src/diff.h @@ -61,5 +61,19 @@ extern bool git_diff_delta__should_skip( extern int git_diff__oid_for_file( git_repository *, const char *, uint16_t, git_off_t, git_oid *); + +GIT_INLINE(bool) git_diff__opts_has_valid_version(const git_diff_options *opts) +{ + if (!opts) + return true; + + if (opts->version > 0 && opts->version <= GIT_DIFF_OPTIONS_VERSION) + return true; + + giterr_set(GITERR_INVALID, "Invalid version %d in git_diff_options", opts->version); + return false; +} + + #endif diff --git a/src/diff_output.c b/src/diff_output.c index e137fd0f2..6316bfa1e 100644 --- a/src/diff_output.c +++ b/src/diff_output.c @@ -1266,6 +1266,9 @@ int git_diff_blobs( git_diff_delta delta; git_diff_patch patch; + if (!git_diff__opts_has_valid_version(options)) + return -1; + if (options && (options->flags & GIT_DIFF_REVERSE)) { git_blob *swap = old_blob; old_blob = new_blob; diff --git a/src/stash.c b/src/stash.c index edd8c55db..261d3b199 100644 --- a/src/stash.c +++ b/src/stash.c @@ -229,7 +229,7 @@ static int build_untracked_tree( { git_tree *i_tree = NULL; git_diff_list *diff = NULL; - git_diff_options opts = {0}; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; struct cb_data data = {0}; int error = -1; @@ -315,7 +315,7 @@ static int build_workdir_tree( git_repository *repo = git_index_owner(index); git_tree *b_tree = NULL; git_diff_list *diff = NULL, *diff2 = NULL; - git_diff_options opts = {0}; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; struct cb_data data = {0}; int error = -1; diff --git a/src/status.c b/src/status.c index c7dea2c71..ca500c5cb 100644 --- a/src/status.c +++ b/src/status.c @@ -108,7 +108,7 @@ int git_status_foreach_ext( void *payload) { int err = 0; - git_diff_options diffopt; + git_diff_options diffopt = GIT_DIFF_OPTIONS_INIT; git_diff_list *idx2head = NULL, *wd2idx = NULL; git_tree *head = NULL; git_status_show_t show = @@ -126,7 +126,6 @@ int git_status_foreach_ext( !(err == GIT_ENOTFOUND || err == GIT_EORPHANEDHEAD)) return err; - memset(&diffopt, 0, sizeof(diffopt)); memcpy(&diffopt.pathspec, &opts->pathspec, sizeof(diffopt.pathspec)); diffopt.flags = GIT_DIFF_INCLUDE_TYPECHANGE; diff --git a/src/submodule.c b/src/submodule.c index c117255d4..7ac666c73 100644 --- a/src/submodule.c +++ b/src/submodule.c @@ -1439,7 +1439,7 @@ static int submodule_wd_status(unsigned int *status, git_submodule *sm) if (sm_repo != NULL) { git_tree *sm_head; - git_diff_options opt; + git_diff_options opt = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff; /* the diffs below could be optimized with an early termination @@ -1452,7 +1452,6 @@ static int submodule_wd_status(unsigned int *status, git_submodule *sm) if ((error = git_repository_head_tree(&sm_head, sm_repo)) < 0) return error; - memset(&opt, 0, sizeof(opt)); if (sm->ignore == GIT_SUBMODULE_IGNORE_NONE) opt.flags |= GIT_DIFF_INCLUDE_UNTRACKED; diff --git a/tests-clar/diff/blob.c b/tests-clar/diff/blob.c index 6a5645d4b..ae3129c9f 100644 --- a/tests-clar/diff/blob.c +++ b/tests-clar/diff/blob.c @@ -12,7 +12,7 @@ void test_diff_blob__initialize(void) g_repo = cl_git_sandbox_init("attr"); - memset(&opts, 0, sizeof(opts)); + reset_diff_opts(&opts); opts.context_lines = 1; opts.interhunk_lines = 0; @@ -313,3 +313,25 @@ void test_diff_blob__comparing_two_text_blobs_honors_interhunkcontext(void) git_blob_free(old_d); } + +void test_diff_blob__checks_options_version_too_low(void) +{ + const git_error *err; + + opts.version = 0; + cl_git_fail(git_diff_blobs( + d, alien, &opts, diff_file_cb, diff_hunk_cb, diff_line_cb, &expected)); + err = giterr_last(); + cl_assert_equal_i(GITERR_INVALID, err->klass); +} + +void test_diff_blob__checks_options_version_too_high(void) +{ + const git_error *err; + + opts.version = 1024; + cl_git_fail(git_diff_blobs( + d, alien, &opts, diff_file_cb, diff_hunk_cb, diff_line_cb, &expected)); + err = giterr_last(); + cl_assert_equal_i(GITERR_INVALID, err->klass); +} diff --git a/tests-clar/diff/diff_helpers.h b/tests-clar/diff/diff_helpers.h index 12591f63e..0f7c0e4f8 100644 --- a/tests-clar/diff/diff_helpers.h +++ b/tests-clar/diff/diff_helpers.h @@ -48,3 +48,10 @@ extern int diff_foreach_via_iterator( void *data); extern void diff_print(FILE *fp, git_diff_list *diff); + + +GIT_INLINE(void) reset_diff_opts(git_diff_options *opts) +{ + git_diff_options init = GIT_DIFF_OPTIONS_INIT; + memmove(opts, &init, sizeof(init)); +} diff --git a/tests-clar/diff/diffiter.c b/tests-clar/diff/diffiter.c index 133392c21..306a13eb9 100644 --- a/tests-clar/diff/diffiter.c +++ b/tests-clar/diff/diffiter.c @@ -76,7 +76,7 @@ void test_diff_diffiter__iterate_files_2(void) void test_diff_diffiter__iterate_files_and_hunks(void) { git_repository *repo = cl_git_sandbox_init("status"); - git_diff_options opts = {0}; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff = NULL; size_t d, num_d; int file_count = 0, hunk_count = 0; @@ -129,7 +129,7 @@ void test_diff_diffiter__iterate_files_and_hunks(void) void test_diff_diffiter__max_size_threshold(void) { git_repository *repo = cl_git_sandbox_init("status"); - git_diff_options opts = {0}; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff = NULL; int file_count = 0, binary_count = 0, hunk_count = 0; size_t d, num_d; @@ -207,7 +207,7 @@ void test_diff_diffiter__max_size_threshold(void) void test_diff_diffiter__iterate_all(void) { git_repository *repo = cl_git_sandbox_init("status"); - git_diff_options opts = {0}; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff = NULL; diff_expects exp = {0}; size_t d, num_d; @@ -280,7 +280,7 @@ static void iterate_over_patch(git_diff_patch *patch, diff_expects *exp) void test_diff_diffiter__iterate_randomly_while_saving_state(void) { git_repository *repo = cl_git_sandbox_init("status"); - git_diff_options opts = {0}; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff = NULL; diff_expects exp = {0}; git_diff_patch *patches[PATCH_CACHE]; @@ -441,3 +441,25 @@ void test_diff_diffiter__iterate_and_generate_patch_text(void) git_diff_list_free(diff); } + +void test_diff_diffiter__checks_options_version(void) +{ + git_repository *repo = cl_git_sandbox_init("status"); + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; + git_diff_list *diff = NULL; + const git_error *err; + + opts.version = 0; + opts.flags |= GIT_DIFF_INCLUDE_IGNORED | GIT_DIFF_INCLUDE_UNTRACKED; + + cl_git_fail(git_diff_workdir_to_index(&diff, repo, NULL, &opts)); + err = giterr_last(); + cl_assert_equal_i(GITERR_INVALID, err->klass); + + giterr_clear(); + opts.version = 1024; + cl_git_fail(git_diff_workdir_to_index(&diff, repo, NULL, &opts)); + err = giterr_last(); + cl_assert_equal_i(GITERR_INVALID, err->klass); +} + diff --git a/tests-clar/diff/index.c b/tests-clar/diff/index.c index 9591e3457..267b3291c 100644 --- a/tests-clar/diff/index.c +++ b/tests-clar/diff/index.c @@ -20,7 +20,7 @@ void test_diff_index__0(void) const char *b_commit = "0017bd4ab1ec3"; /* the start */ git_tree *a = resolve_commit_oid_to_tree(g_repo, a_commit); git_tree *b = resolve_commit_oid_to_tree(g_repo, b_commit); - git_diff_options opts = {0}; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff = NULL; diff_expects exp; @@ -113,7 +113,7 @@ void test_diff_index__1(void) const char *b_commit = "0017bd4ab1ec3"; /* the start */ git_tree *a = resolve_commit_oid_to_tree(g_repo, a_commit); git_tree *b = resolve_commit_oid_to_tree(g_repo, b_commit); - git_diff_options opts = {0}; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff = NULL; diff_expects exp; @@ -140,3 +140,24 @@ void test_diff_index__1(void) git_tree_free(a); git_tree_free(b); } + +void test_diff_index__checks_options_version(void) +{ + const char *a_commit = "26a125ee1bf"; + git_tree *a = resolve_commit_oid_to_tree(g_repo, a_commit); + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; + git_diff_list *diff; + const git_error *err; + + opts.version = 0; + cl_git_fail(git_diff_index_to_tree(&diff, g_repo, a, NULL, &opts)); + err = giterr_last(); + cl_assert_equal_i(GITERR_INVALID, err->klass); + + giterr_clear(); + opts.version = 1024; + cl_git_fail(git_diff_index_to_tree(&diff, g_repo, a, NULL, &opts)); + err = giterr_last(); + cl_assert_equal_i(GITERR_INVALID, err->klass); +} + diff --git a/tests-clar/diff/rename.c b/tests-clar/diff/rename.c index 0d57f8ff0..18b7f7719 100644 --- a/tests-clar/diff/rename.c +++ b/tests-clar/diff/rename.c @@ -33,7 +33,7 @@ void test_diff_rename__match_oid(void) const char *new_sha = "2bc7f351d20b53f1c72c16c4b036e491c478c49a"; git_tree *old_tree, *new_tree; git_diff_list *diff; - git_diff_options diffopts = {0}; + git_diff_options diffopts = GIT_DIFF_OPTIONS_INIT; git_diff_find_options opts; diff_expects exp; @@ -43,7 +43,6 @@ void test_diff_rename__match_oid(void) /* Must pass GIT_DIFF_INCLUDE_UNMODIFIED if you expect to emulate * --find-copies-harder during rename transformion... */ - memset(&diffopts, 0, sizeof(diffopts)); diffopts.flags |= GIT_DIFF_INCLUDE_UNMODIFIED; cl_git_pass(git_diff_tree_to_tree( diff --git a/tests-clar/diff/tree.c b/tests-clar/diff/tree.c index 58dc4e6fa..442e53b25 100644 --- a/tests-clar/diff/tree.c +++ b/tests-clar/diff/tree.c @@ -19,7 +19,7 @@ void test_diff_tree__0(void) const char *b_commit = "370fe9ec22"; const char *c_commit = "f5b0af1fb4f5c"; git_tree *a, *b, *c; - git_diff_options opts = {0}; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff = NULL; diff_expects exp; @@ -175,7 +175,7 @@ void test_diff_tree__bare(void) const char *a_commit = "8496071c1b46c85"; const char *b_commit = "be3563ae3f79"; git_tree *a, *b; - git_diff_options opts = {0}; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff = NULL; diff_expects exp; @@ -264,7 +264,7 @@ void test_diff_tree__larger_hunks(void) const char *a_commit = "d70d245ed97ed2aa596dd1af6536e4bfdb047b69"; const char *b_commit = "7a9e0b02e63179929fed24f0a3e0f19168114d10"; git_tree *a, *b; - git_diff_options opts = {0}; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff = NULL; size_t d, num_d, h, num_h, l, num_l, header_len, line_len; const git_diff_delta *delta; @@ -319,3 +319,31 @@ void test_diff_tree__larger_hunks(void) git_tree_free(a); git_tree_free(b); } + +void test_diff_tree__checks_options_version(void) +{ + const char *a_commit = "8496071c1b46c85"; + const char *b_commit = "be3563ae3f79"; + git_tree *a, *b; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; + git_diff_list *diff = NULL; + const git_error *err; + + g_repo = cl_git_sandbox_init("testrepo.git"); + + cl_assert((a = resolve_commit_oid_to_tree(g_repo, a_commit)) != NULL); + cl_assert((b = resolve_commit_oid_to_tree(g_repo, b_commit)) != NULL); + + opts.version = 0; + cl_git_fail(git_diff_tree_to_tree(&diff, g_repo, a, b, &opts)); + err = giterr_last(); + cl_assert_equal_i(GITERR_INVALID, err->klass); + + giterr_clear(); + opts.version = 1024; + cl_git_fail(git_diff_tree_to_tree(&diff, g_repo, a, b, &opts)); + err = giterr_last(); + + git_tree_free(a); + git_tree_free(b); +} diff --git a/tests-clar/diff/workdir.c b/tests-clar/diff/workdir.c index 57c88c3e5..10d58b118 100644 --- a/tests-clar/diff/workdir.c +++ b/tests-clar/diff/workdir.c @@ -15,7 +15,7 @@ void test_diff_workdir__cleanup(void) void test_diff_workdir__to_index(void) { - git_diff_options opts = {0}; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff = NULL; diff_expects exp; int use_iterator; @@ -69,7 +69,7 @@ void test_diff_workdir__to_tree(void) const char *a_commit = "26a125ee1bf"; /* the current HEAD */ const char *b_commit = "0017bd4ab1ec3"; /* the start */ git_tree *a, *b; - git_diff_options opts = {0}; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff = NULL; git_diff_list *diff2 = NULL; diff_expects exp; @@ -202,7 +202,7 @@ void test_diff_workdir__to_tree(void) void test_diff_workdir__to_index_with_pathspec(void) { - git_diff_options opts = {0}; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff = NULL; diff_expects exp; char *pathspec = NULL; @@ -420,7 +420,7 @@ void test_diff_workdir__filemode_changes_with_filemode_false(void) void test_diff_workdir__head_index_and_workdir_all_differ(void) { - git_diff_options opts = {0}; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff_i2t = NULL, *diff_w2i = NULL; diff_expects exp; char *pathspec = "staged_changes_modified_file"; @@ -518,7 +518,7 @@ void test_diff_workdir__head_index_and_workdir_all_differ(void) void test_diff_workdir__eof_newline_changes(void) { - git_diff_options opts = {0}; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff = NULL; diff_expects exp; char *pathspec = "current_file"; @@ -678,7 +678,7 @@ void test_diff_workdir__larger_hunks(void) const char *a_commit = "d70d245ed97ed2aa596dd1af6536e4bfdb047b69"; const char *b_commit = "7a9e0b02e63179929fed24f0a3e0f19168114d10"; git_tree *a, *b; - git_diff_options opts = {0}; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; size_t i, d, num_d, h, num_h, l, num_l, header_len, line_len; g_repo = cl_git_sandbox_init("diff"); @@ -763,7 +763,7 @@ void test_diff_workdir__submodules(void) { const char *a_commit = "873585b94bdeabccea991ea5e3ec1a277895b698"; git_tree *a; - git_diff_options opts = {0}; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff = NULL; diff_expects exp; @@ -822,7 +822,7 @@ void test_diff_workdir__submodules(void) void test_diff_workdir__cannot_diff_against_a_bare_repository(void) { - git_diff_options opts = {0}; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff = NULL; git_tree *tree; @@ -843,7 +843,7 @@ void test_diff_workdir__to_null_tree(void) { git_diff_list *diff; diff_expects exp; - git_diff_options opts = {0}; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; opts.flags = GIT_DIFF_INCLUDE_UNTRACKED | GIT_DIFF_RECURSE_UNTRACKED_DIRS; @@ -861,3 +861,23 @@ void test_diff_workdir__to_null_tree(void) git_diff_list_free(diff); } + +void test_diff_workdir__checks_options_version(void) +{ + git_diff_list *diff; + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; + const git_error *err; + + g_repo = cl_git_sandbox_init("status"); + + opts.version = 0; + cl_git_fail(git_diff_workdir_to_tree(&diff, g_repo, NULL, &opts)); + err = giterr_last(); + cl_assert_equal_i(GITERR_INVALID, err->klass); + + giterr_clear(); + opts.version = 1024; + cl_git_fail(git_diff_workdir_to_tree(&diff, g_repo, NULL, &opts)); + err = giterr_last(); + cl_assert_equal_i(GITERR_INVALID, err->klass); +} From ca901e7b0fb38a7f4748ff5fcde7a2779f3a7771 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Thu, 29 Nov 2012 15:16:19 -0800 Subject: [PATCH 06/18] Deploy GIT_DIFF_FIND_OPTIONS_INIT --- src/diff_tform.c | 18 +++++++++++++++++- tests-clar/diff/rename.c | 35 +++++++++++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/diff_tform.c b/src/diff_tform.c index 987d4b8e6..1df041044 100644 --- a/src/diff_tform.c +++ b/src/diff_tform.c @@ -168,6 +168,18 @@ int git_diff_merge( return error; } +static bool find_opts_has_valid_version(const git_diff_find_options *opts) +{ + if (!opts) + return true; + + if (opts->version > 0 && opts->version <= GIT_DIFF_FIND_OPTIONS_VERSION) + return true; + + giterr_set(GITERR_INVALID, "Invalid version %d on git_diff_find_options", opts->version); + return false; +} + #define DEFAULT_THRESHOLD 50 #define DEFAULT_BREAK_REWRITE_THRESHOLD 60 #define DEFAULT_TARGET_LIMIT 200 @@ -187,7 +199,8 @@ static int normalize_find_opts( if (given != NULL) memcpy(opts, given, sizeof(*opts)); else { - memset(opts, 0, sizeof(*opts)); + git_diff_find_options init = GIT_DIFF_FIND_OPTIONS_INIT; + memmove(opts, &init, sizeof(init)); opts->flags = GIT_DIFF_FIND_RENAMES; @@ -198,6 +211,9 @@ static int normalize_find_opts( opts->flags = GIT_DIFF_FIND_RENAMES | GIT_DIFF_FIND_COPIES; } + if (!find_opts_has_valid_version(opts)) + return -1; + /* some flags imply others */ if (opts->flags & GIT_DIFF_FIND_RENAMES_FROM_REWRITES) diff --git a/tests-clar/diff/rename.c b/tests-clar/diff/rename.c index 18b7f7719..2995b4ef5 100644 --- a/tests-clar/diff/rename.c +++ b/tests-clar/diff/rename.c @@ -34,7 +34,7 @@ void test_diff_rename__match_oid(void) git_tree *old_tree, *new_tree; git_diff_list *diff; git_diff_options diffopts = GIT_DIFF_OPTIONS_INIT; - git_diff_find_options opts; + git_diff_find_options opts = GIT_DIFF_FIND_OPTIONS_INIT; diff_expects exp; old_tree = resolve_commit_oid_to_tree(g_repo, old_sha); @@ -84,7 +84,6 @@ void test_diff_rename__match_oid(void) * 31e47d8c1fa36d7f8d537b96158e3f024de0a9f2 \ * 2bc7f351d20b53f1c72c16c4b036e491c478c49a */ - memset(&opts, 0, sizeof(opts)); opts.flags = GIT_DIFF_FIND_COPIES_FROM_UNMODIFIED; cl_git_pass(git_diff_find_similar(diff, &opts)); @@ -102,3 +101,35 @@ void test_diff_rename__match_oid(void) git_tree_free(old_tree); git_tree_free(new_tree); } + +void test_diff_rename__checks_options_version(void) +{ + const char *old_sha = "31e47d8c1fa36d7f8d537b96158e3f024de0a9f2"; + const char *new_sha = "2bc7f351d20b53f1c72c16c4b036e491c478c49a"; + git_tree *old_tree, *new_tree; + git_diff_list *diff; + git_diff_options diffopts = GIT_DIFF_OPTIONS_INIT; + git_diff_find_options opts = GIT_DIFF_FIND_OPTIONS_INIT; + const git_error *err; + + old_tree = resolve_commit_oid_to_tree(g_repo, old_sha); + new_tree = resolve_commit_oid_to_tree(g_repo, new_sha); + diffopts.flags |= GIT_DIFF_INCLUDE_UNMODIFIED; + cl_git_pass(git_diff_tree_to_tree( + &diff, g_repo, old_tree, new_tree, &diffopts)); + + opts.version = 0; + cl_git_fail(git_diff_find_similar(diff, &opts)); + err = giterr_last(); + cl_assert_equal_i(GITERR_INVALID, err->klass); + + giterr_clear(); + opts.version = 1024; + cl_git_fail(git_diff_find_similar(diff, &opts)); + err = giterr_last(); + cl_assert_equal_i(GITERR_INVALID, err->klass); + + git_diff_list_free(diff); + git_tree_free(old_tree); + git_tree_free(new_tree); +} From 55f6f21b7dd5a177e2cc60c1054b438c05ed8f14 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Thu, 29 Nov 2012 19:59:18 -0800 Subject: [PATCH 07/18] Deploy versioned git_odb_backend structure --- src/odb.c | 15 +++++++++++++++ src/odb_loose.c | 1 + src/odb_pack.c | 2 ++ tests-clar/odb/sorting.c | 4 ++-- 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/odb.c b/src/odb.c index 63b68284a..e35e4caca 100644 --- a/src/odb.c +++ b/src/odb.c @@ -363,12 +363,27 @@ int git_odb_new(git_odb **out) return 0; } +static bool backend_has_valid_version(git_odb_backend *backend) +{ + if (!backend) + return true; + + if (backend->version > 0 && backend->version <= GIT_ODB_BACKEND_VERSION) + return true; + + giterr_set(GITERR_INVALID, "Invalid version %d on git_odb_backend", backend->version); + return false; +} + static int add_backend_internal(git_odb *odb, git_odb_backend *backend, int priority, int is_alternate) { backend_internal *internal; assert(odb && backend); + if (!backend_has_valid_version(backend)) + return -1; + /* Check if the backend is already owned by another ODB */ assert(!backend->odb || backend->odb == odb); diff --git a/src/odb_loose.c b/src/odb_loose.c index e2f1aec32..df86d903e 100644 --- a/src/odb_loose.c +++ b/src/odb_loose.c @@ -915,6 +915,7 @@ int git_odb_backend_loose( backend = git__calloc(1, sizeof(loose_backend)); GITERR_CHECK_ALLOC(backend); + backend->parent.version = GIT_ODB_BACKEND_VERSION; backend->objects_dir = git__strdup(objects_dir); GITERR_CHECK_ALLOC(backend->objects_dir); diff --git a/src/odb_pack.c b/src/odb_pack.c index 35bf1580d..41789e4ec 100644 --- a/src/odb_pack.c +++ b/src/odb_pack.c @@ -570,6 +570,7 @@ int git_odb_backend_one_pack(git_odb_backend **backend_out, const char *idx) backend = git__calloc(1, sizeof(struct pack_backend)); GITERR_CHECK_ALLOC(backend); + backend->parent.version = GIT_ODB_BACKEND_VERSION; if (git_vector_init(&backend->packs, 1, NULL) < 0) goto on_error; @@ -602,6 +603,7 @@ int git_odb_backend_pack(git_odb_backend **backend_out, const char *objects_dir) backend = git__calloc(1, sizeof(struct pack_backend)); GITERR_CHECK_ALLOC(backend); + backend->parent.version = GIT_ODB_BACKEND_VERSION; if (git_vector_init(&backend->packs, 8, packfile_sort__cb) < 0 || git_buf_joinpath(&path, objects_dir, "pack") < 0) diff --git a/tests-clar/odb/sorting.c b/tests-clar/odb/sorting.c index bf64f6af4..b4f9e44bc 100644 --- a/tests-clar/odb/sorting.c +++ b/tests-clar/odb/sorting.c @@ -11,11 +11,11 @@ static git_odb_backend *new_backend(int position) { fake_backend *b; - b = git__malloc(sizeof(fake_backend)); + b = git__calloc(1, sizeof(fake_backend)); if (b == NULL) return NULL; - memset(b, 0x0, sizeof(fake_backend)); + b->base.version = GIT_ODB_BACKEND_VERSION; b->position = position; return (git_odb_backend *)b; } From 9267ff586f29deb99f41ab6eeb0599917ddd0cec Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Thu, 29 Nov 2012 20:01:24 -0800 Subject: [PATCH 08/18] Deploy GIT_REMOTE_CALLBACKS_INIT --- examples/network/fetch.c | 3 +-- include/git2/remote.h | 3 ++- src/remote.c | 19 ++++++++++++++++++- tests-clar/network/fetch.c | 3 +-- tests-clar/network/push_util.h | 3 ++- 5 files changed, 24 insertions(+), 7 deletions(-) diff --git a/examples/network/fetch.c b/examples/network/fetch.c index e341d2d6c..8048fd67a 100644 --- a/examples/network/fetch.c +++ b/examples/network/fetch.c @@ -70,7 +70,7 @@ int fetch(git_repository *repo, int argc, char **argv) const git_transfer_progress *stats; pthread_t worker; struct dl_data data; - git_remote_callbacks callbacks; + git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT; argc = argc; // Figure out whether it's a named remote or a URL @@ -81,7 +81,6 @@ int fetch(git_repository *repo, int argc, char **argv) } // Set up the callbacks (only update_tips for now) - memset(&callbacks, 0, sizeof(callbacks)); callbacks.update_tips = &update_cb; callbacks.progress = &progress_cb; git_remote_set_callbacks(remote, &callbacks); diff --git a/include/git2/remote.h b/include/git2/remote.h index e5b60b951..3e22004a1 100644 --- a/include/git2/remote.h +++ b/include/git2/remote.h @@ -356,8 +356,9 @@ struct git_remote_callbacks { * * @param remote the remote to configure * @param callbacks a pointer to the user's callback settings + * @return 0 or an error code */ -GIT_EXTERN(void) git_remote_set_callbacks(git_remote *remote, git_remote_callbacks *callbacks); +GIT_EXTERN(int) git_remote_set_callbacks(git_remote *remote, git_remote_callbacks *callbacks); /** * Get the statistics structure that is filled in by the fetch operation. diff --git a/src/remote.c b/src/remote.c index c84911aa1..d516d07fb 100644 --- a/src/remote.c +++ b/src/remote.c @@ -985,10 +985,25 @@ void git_remote_check_cert(git_remote *remote, int check) remote->check_cert = check; } -void git_remote_set_callbacks(git_remote *remote, git_remote_callbacks *callbacks) +static bool callbacks_have_valid_version(git_remote_callbacks *callbacks) +{ + if (!callbacks) + return true; + + if (callbacks->version > 0 && callbacks->version <= GIT_REMOTE_CALLBACKS_VERSION) + return true; + + giterr_set(GITERR_INVALID, "Invalid version %d for git_remote_callbacks", callbacks->version); + return false; +} + +int git_remote_set_callbacks(git_remote *remote, git_remote_callbacks *callbacks) { assert(remote && callbacks); + if (!callbacks_have_valid_version(callbacks)) + return -1; + memcpy(&remote->callbacks, callbacks, sizeof(git_remote_callbacks)); if (remote->transport && remote->transport->set_callbacks) @@ -996,6 +1011,8 @@ void git_remote_set_callbacks(git_remote *remote, git_remote_callbacks *callback remote->callbacks.progress, NULL, remote->callbacks.payload); + + return 0; } void git_remote_set_cred_acquire_cb( diff --git a/tests-clar/network/fetch.c b/tests-clar/network/fetch.c index 26937c608..859479e6e 100644 --- a/tests-clar/network/fetch.c +++ b/tests-clar/network/fetch.c @@ -36,10 +36,9 @@ static void progress(const git_transfer_progress *stats, void *payload) static void do_fetch(const char *url, git_remote_autotag_option_t flag, int n) { git_remote *remote; - git_remote_callbacks callbacks; + git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT; size_t bytes_received = 0; - memset(&callbacks, 0, sizeof(git_remote_callbacks)); callbacks.update_tips = update_tips; counter = 0; diff --git a/tests-clar/network/push_util.h b/tests-clar/network/push_util.h index 2f4dffce4..759122aa6 100644 --- a/tests-clar/network/push_util.h +++ b/tests-clar/network/push_util.h @@ -11,7 +11,8 @@ extern const git_oid OID_ZERO; * record data in a record_callbacks_data instance. * @param data pointer to a record_callbacks_data instance */ -#define RECORD_CALLBACKS_INIT(data) { NULL, NULL, record_update_tips_cb, data } +#define RECORD_CALLBACKS_INIT(data) \ + { GIT_REMOTE_CALLBACKS_VERSION, NULL, NULL, record_update_tips_cb, data } typedef struct { char *name; From b4d136527c2210f1abbcc53ab7290ad38029ab3c Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Thu, 29 Nov 2012 20:06:23 -0800 Subject: [PATCH 09/18] Deploy GIT_REPOSITORY_INIT_OPTIONS_INIT --- src/repository.c | 18 ++++++++++++++++-- src/submodule.c | 3 +-- tests-clar/repo/init.c | 9 +++------ 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/repository.c b/src/repository.c index b49b49b7a..073efa484 100644 --- a/src/repository.c +++ b/src/repository.c @@ -1151,9 +1151,8 @@ static int repo_init_create_origin(git_repository *repo, const char *url) int git_repository_init( git_repository **repo_out, const char *path, unsigned is_bare) { - git_repository_init_options opts; + git_repository_init_options opts = GIT_REPOSITORY_INIT_OPTIONS_INIT; - memset(&opts, 0, sizeof(opts)); opts.flags = GIT_REPOSITORY_INIT_MKPATH; /* don't love this default */ if (is_bare) opts.flags |= GIT_REPOSITORY_INIT_BARE; @@ -1161,6 +1160,18 @@ int git_repository_init( return git_repository_init_ext(repo_out, path, &opts); } +static bool options_have_valid_version(git_repository_init_options *opts) +{ + if (!opts) + return true; + + if (opts->version > 0 && opts->version <= GIT_REMOTE_CALLBACKS_VERSION) + return true; + + giterr_set(GITERR_INVALID, "Invalid version %d for git_repository_init_options", opts->version); + return false; +} + int git_repository_init_ext( git_repository **out, const char *given_repo, @@ -1171,6 +1182,9 @@ int git_repository_init_ext( assert(out && given_repo && opts); + if (!options_have_valid_version(opts)) + return -1; + error = repo_init_directories(&repo_path, &wd_path, given_repo, opts); if (error < 0) goto cleanup; diff --git a/src/submodule.c b/src/submodule.c index 7ac666c73..21a1875c2 100644 --- a/src/submodule.c +++ b/src/submodule.c @@ -205,7 +205,7 @@ int git_submodule_add_setup( git_config_backend *mods = NULL; git_submodule *sm; git_buf name = GIT_BUF_INIT, real_url = GIT_BUF_INIT; - git_repository_init_options initopt; + git_repository_init_options initopt = GIT_REPOSITORY_INIT_OPTIONS_INIT; git_repository *subrepo = NULL; assert(repo && url && path); @@ -275,7 +275,6 @@ int git_submodule_add_setup( * Old style: sub-repo goes directly into repo//.git/ */ - memset(&initopt, 0, sizeof(initopt)); initopt.flags = GIT_REPOSITORY_INIT_MKPATH | GIT_REPOSITORY_INIT_NO_REINIT; initopt.origin_url = real_url.ptr; diff --git a/tests-clar/repo/init.c b/tests-clar/repo/init.c index 3b14c97f2..c0acbed5a 100644 --- a/tests-clar/repo/init.c +++ b/tests-clar/repo/init.c @@ -304,8 +304,7 @@ void test_repo_init__sets_logAllRefUpdates_according_to_type_of_repository(void) void test_repo_init__extended_0(void) { - git_repository_init_options opts; - memset(&opts, 0, sizeof(opts)); + git_repository_init_options opts = GIT_REPOSITORY_INIT_OPTIONS_INIT; /* without MKDIR this should fail */ cl_git_fail(git_repository_init_ext(&_repo, "extended", &opts)); @@ -327,8 +326,7 @@ void test_repo_init__extended_1(void) git_reference *ref; git_remote *remote; struct stat st; - git_repository_init_options opts; - memset(&opts, 0, sizeof(opts)); + git_repository_init_options opts = GIT_REPOSITORY_INIT_OPTIONS_INIT; opts.flags = GIT_REPOSITORY_INIT_MKPATH | GIT_REPOSITORY_INIT_NO_DOTGIT_DIR; @@ -367,8 +365,7 @@ void test_repo_init__extended_1(void) void test_repo_init__extended_with_template(void) { - git_repository_init_options opts; - memset(&opts, 0, sizeof(opts)); + git_repository_init_options opts = GIT_REPOSITORY_INIT_OPTIONS_INIT; opts.flags = GIT_REPOSITORY_INIT_MKPATH | GIT_REPOSITORY_INIT_BARE; opts.template_path = cl_fixture("template"); From 79cfa20d60cfdaf578da59cfb4d17551cf1b6256 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Thu, 29 Nov 2012 20:12:59 -0800 Subject: [PATCH 10/18] Deploy GIT_STATUS_OPTIONS_INIT --- src/stash.c | 3 +-- src/status.c | 24 ++++++++++++++++++------ tests-clar/status/ignore.c | 8 -------- tests-clar/status/worktree.c | 12 ++++-------- 4 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/stash.c b/src/stash.c index 261d3b199..98c7a7c93 100644 --- a/src/stash.c +++ b/src/stash.c @@ -471,9 +471,8 @@ static int ensure_there_are_changes_to_stash( bool include_ignored_files) { int error; - git_status_options opts; + git_status_options opts = GIT_STATUS_OPTIONS_INIT; - memset(&opts, 0, sizeof(opts)); opts.show = GIT_STATUS_SHOW_INDEX_AND_WORKDIR; if (include_untracked_files) opts.flags = GIT_STATUS_OPT_INCLUDE_UNTRACKED | diff --git a/src/status.c b/src/status.c index ca500c5cb..f23f40e6d 100644 --- a/src/status.c +++ b/src/status.c @@ -101,6 +101,18 @@ static int status_invoke_cb( return usercb->cb(path, status, usercb->payload); } +static bool options_have_valid_version(const git_status_options *opts) +{ + if (!opts) + return true; + + if (opts->version > 0 && opts->version <= GIT_REMOTE_CALLBACKS_VERSION) + return true; + + giterr_set(GITERR_INVALID, "Invalid version %d for git_repository_init_options", opts->version); + return false; +} + int git_status_foreach_ext( git_repository *repo, const git_status_options *opts, @@ -117,6 +129,9 @@ int git_status_foreach_ext( assert(show <= GIT_STATUS_SHOW_INDEX_THEN_WORKDIR); + if (!options_have_valid_version(opts)) + return -1; + if (show != GIT_STATUS_SHOW_INDEX_ONLY && (err = git_repository__ensure_not_bare(repo, "status")) < 0) return err; @@ -180,9 +195,8 @@ int git_status_foreach( git_status_cb callback, void *payload) { - git_status_options opts; + git_status_options opts = GIT_STATUS_OPTIONS_INIT; - memset(&opts, 0, sizeof(opts)); opts.show = GIT_STATUS_SHOW_INDEX_AND_WORKDIR; opts.flags = GIT_STATUS_OPT_INCLUDE_IGNORED | GIT_STATUS_OPT_INCLUDE_UNTRACKED | @@ -223,16 +237,14 @@ int git_status_file( const char *path) { int error; - git_status_options opts; - struct status_file_info sfi; + git_status_options opts = GIT_STATUS_OPTIONS_INIT; + struct status_file_info sfi = {0}; assert(status_flags && repo && path); - memset(&sfi, 0, sizeof(sfi)); if ((sfi.expected = git__strdup(path)) == NULL) return -1; - memset(&opts, 0, sizeof(opts)); opts.show = GIT_STATUS_SHOW_INDEX_AND_WORKDIR; opts.flags = GIT_STATUS_OPT_INCLUDE_IGNORED | GIT_STATUS_OPT_INCLUDE_UNTRACKED | diff --git a/tests-clar/status/ignore.c b/tests-clar/status/ignore.c index 27f9d85b9..e2e4aaf18 100644 --- a/tests-clar/status/ignore.c +++ b/tests-clar/status/ignore.c @@ -180,9 +180,6 @@ void test_status_ignore__subdirectories(void) { status_entry_single st; int ignored; - git_status_options opts; - - GIT_UNUSED(opts); g_repo = cl_git_sandbox_init("empty_standard_repo"); @@ -216,11 +213,6 @@ void test_status_ignore__subdirectories(void) cl_git_mkfile( "empty_standard_repo/test/ignore_me/file", "I'm going to be ignored!"); - opts.show = GIT_STATUS_SHOW_INDEX_AND_WORKDIR; - opts.flags = GIT_STATUS_OPT_INCLUDE_IGNORED | - GIT_STATUS_OPT_INCLUDE_UNTRACKED | - GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS; - memset(&st, 0, sizeof(st)); cl_git_pass(git_status_foreach(g_repo, cb_status__single, &st)); cl_assert_equal_i(2, st.count); diff --git a/tests-clar/status/worktree.c b/tests-clar/status/worktree.c index 838a04377..6f5fb3c85 100644 --- a/tests-clar/status/worktree.c +++ b/tests-clar/status/worktree.c @@ -111,7 +111,7 @@ void test_status_worktree__swap_subdir_and_file(void) status_entry_counts counts; git_repository *repo = cl_git_sandbox_init("status"); git_index *index; - git_status_options opts; + git_status_options opts = GIT_STATUS_OPTIONS_INIT; bool ignore_case; cl_git_pass(git_repository_index(&index, repo)); @@ -133,7 +133,6 @@ void test_status_worktree__swap_subdir_and_file(void) counts.expected_paths = ignore_case ? entry_paths3_icase : entry_paths3; counts.expected_statuses = ignore_case ? entry_statuses3_icase : entry_statuses3; - memset(&opts, 0, sizeof(opts)); opts.flags = GIT_STATUS_OPT_INCLUDE_UNTRACKED | GIT_STATUS_OPT_INCLUDE_IGNORED; @@ -150,7 +149,7 @@ void test_status_worktree__swap_subdir_with_recurse_and_pathspec(void) { status_entry_counts counts; git_repository *repo = cl_git_sandbox_init("status"); - git_status_options opts; + git_status_options opts = GIT_STATUS_OPTIONS_INIT; /* first alter the contents of the worktree */ cl_git_pass(p_rename("status/current_file", "status/swap")); @@ -167,7 +166,6 @@ void test_status_worktree__swap_subdir_with_recurse_and_pathspec(void) counts.expected_paths = entry_paths4; counts.expected_statuses = entry_statuses4; - memset(&opts, 0, sizeof(opts)); opts.flags = GIT_STATUS_OPT_INCLUDE_UNTRACKED | GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS; /* TODO: set pathspec to "current_file" eventually */ @@ -691,7 +689,7 @@ void test_status_worktree__filemode_changes(void) { git_repository *repo = cl_git_sandbox_init("filemodes"); status_entry_counts counts; - git_status_options opts; + git_status_options opts = GIT_STATUS_OPTIONS_INIT; git_config *cfg; /* overwrite stored filemode with platform appropriate value */ @@ -708,7 +706,6 @@ void test_status_worktree__filemode_changes(void) filemode_statuses[i] = GIT_STATUS_CURRENT; } - memset(&opts, 0, sizeof(opts)); opts.flags = GIT_STATUS_OPT_INCLUDE_UNTRACKED | GIT_STATUS_OPT_INCLUDE_IGNORED | GIT_STATUS_OPT_INCLUDE_UNMODIFIED; @@ -746,7 +743,7 @@ static int cb_status__expected_path(const char *p, unsigned int s, void *payload void test_status_worktree__disable_pathspec_match(void) { git_repository *repo; - git_status_options opts; + git_status_options opts = GIT_STATUS_OPTIONS_INIT; char *file_with_bracket = "LICENSE[1].md", *imaginary_file_with_bracket = "LICENSE[1-2].md"; @@ -754,7 +751,6 @@ void test_status_worktree__disable_pathspec_match(void) cl_git_mkfile("pathspec/LICENSE[1].md", "screaming bracket\n"); cl_git_mkfile("pathspec/LICENSE1.md", "no bracket\n"); - memset(&opts, 0, sizeof(opts)); opts.flags = GIT_STATUS_OPT_INCLUDE_UNTRACKED | GIT_STATUS_OPT_DISABLE_PATHSPEC_MATCH; opts.pathspec.count = 1; From 10711769000d009189f83e468f25b719fa303086 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Thu, 29 Nov 2012 20:47:37 -0800 Subject: [PATCH 11/18] Deploy versioned git_transport structure --- src/remote.c | 15 +++++++++++++++ src/transports/local.c | 5 ++--- src/transports/smart.c | 1 + tests-clar/network/remotes.c | 20 ++++++++++++++++++++ 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/remote.c b/src/remote.c index d516d07fb..5fc6b4f35 100644 --- a/src/remote.c +++ b/src/remote.c @@ -1024,10 +1024,25 @@ void git_remote_set_cred_acquire_cb( remote->cred_acquire_cb = cred_acquire_cb; } +static bool transport_has_valid_version(const git_transport *transport) +{ + if (!transport) + return true; + + if (transport->version > 0 && transport->version <= GIT_TRANSPORT_VERSION) + return true; + + giterr_set(GITERR_INVALID, "Invalid version %d on git_transport", transport->version); + return false; +} + int git_remote_set_transport(git_remote *remote, git_transport *transport) { assert(remote && transport); + if (!transport_has_valid_version(transport)) + return -1; + if (remote->transport) { giterr_set(GITERR_NET, "A transport is already bound to this remote"); return -1; diff --git a/src/transports/local.c b/src/transports/local.c index 62e8024b5..768daf3a8 100644 --- a/src/transports/local.c +++ b/src/transports/local.c @@ -403,11 +403,10 @@ int git_transport_local(git_transport **out, git_remote *owner, void *param) GIT_UNUSED(param); - t = git__malloc(sizeof(transport_local)); + t = git__calloc(1, sizeof(transport_local)); GITERR_CHECK_ALLOC(t); - memset(t, 0x0, sizeof(transport_local)); - + t->parent.version = GIT_TRANSPORT_VERSION; t->parent.connect = local_connect; t->parent.negotiate_fetch = local_negotiate_fetch; t->parent.download_pack = local_download_pack; diff --git a/src/transports/smart.c b/src/transports/smart.c index 94d389b52..5300a47c8 100644 --- a/src/transports/smart.c +++ b/src/transports/smart.c @@ -303,6 +303,7 @@ int git_transport_smart(git_transport **out, git_remote *owner, void *param) t = git__calloc(sizeof(transport_smart), 1); GITERR_CHECK_ALLOC(t); + t->parent.version = GIT_TRANSPORT_VERSION; t->parent.set_callbacks = git_smart__set_callbacks; t->parent.connect = git_smart__connect; t->parent.close = git_smart__close; diff --git a/tests-clar/network/remotes.c b/tests-clar/network/remotes.c index 14fda1670..17f070470 100644 --- a/tests-clar/network/remotes.c +++ b/tests-clar/network/remotes.c @@ -279,3 +279,23 @@ void test_network_remotes__cannot_load_with_an_empty_url(void) cl_git_fail(git_remote_load(&remote, _repo, "empty-remote-url")); cl_assert(giterr_last()->klass == GITERR_INVALID); } + +void test_network_remotes__check_structure_version(void) +{ + git_transport transport = GIT_TRANSPORT_INIT; + const git_error *err; + + git_remote_free(_remote); + cl_git_pass(git_remote_new(&_remote, _repo, NULL, "test-protocol://localhost", NULL)); + + transport.version = 0; + cl_git_fail(git_remote_set_transport(_remote, &transport)); + err = giterr_last(); + cl_assert_equal_i(GITERR_INVALID, err->klass); + + giterr_clear(); + transport.version = 1024; + cl_git_fail(git_remote_set_transport(_remote, &transport)); + err = giterr_last(); + cl_assert_equal_i(GITERR_INVALID, err->klass); +} From 4ec197f3049d203739066e0c2d2c5c39f78fd808 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Fri, 30 Nov 2012 12:52:42 -0800 Subject: [PATCH 12/18] Deploy GIT_SIGNATURE_INIT --- src/commit.c | 3 +++ src/notes.c | 9 +++++++++ src/reflog.c | 4 ++++ src/signature.c | 4 +++- src/signature.h | 12 ++++++++++++ src/stash.c | 4 ++++ src/tag.c | 3 +++ 7 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/commit.c b/src/commit.c index 4072518ff..5197fdb6d 100644 --- a/src/commit.c +++ b/src/commit.c @@ -93,6 +93,9 @@ int git_commit_create( git_odb *odb; assert(git_object_owner((const git_object *)tree) == repo); + if (!git_signature__has_valid_version(author) || + !git_signature__has_valid_version(committer)) + return -1; git_oid__writebuf(&commit, "tree ", git_object_id((const git_object *)tree)); diff --git a/src/notes.c b/src/notes.c index dd36cc2fe..87ee1e742 100644 --- a/src/notes.c +++ b/src/notes.c @@ -11,6 +11,7 @@ #include "refs.h" #include "config.h" #include "iterator.h" +#include "signature.h" static int find_subtree_in_current_level( git_tree **out, @@ -455,6 +456,10 @@ int git_note_create( git_commit *commit = NULL; git_tree *tree = NULL; + if (!git_signature__has_valid_version(author) || + !git_signature__has_valid_version(committer)) + return -1; + target = git_oid_allocfmt(oid); GITERR_CHECK_ALLOC(target); @@ -482,6 +487,10 @@ int git_note_remove(git_repository *repo, const char *notes_ref, git_commit *commit = NULL; git_tree *tree = NULL; + if (!git_signature__has_valid_version(author) || + !git_signature__has_valid_version(committer)) + return -1; + target = git_oid_allocfmt(oid); GITERR_CHECK_ALLOC(target); diff --git a/src/reflog.c b/src/reflog.c index ac481fb81..4d119bae7 100644 --- a/src/reflog.c +++ b/src/reflog.c @@ -112,6 +112,7 @@ static int reflog_parse(git_reflog *log, const char *buf, size_t buf_size) entry->committer = git__malloc(sizeof(git_signature)); GITERR_CHECK_ALLOC(entry->committer); + entry->committer->version = GIT_SIGNATURE_VERSION; if (git_oid_fromstrn(&entry->oid_old, buf, GIT_OID_HEXSZ) < 0) goto fail; @@ -297,6 +298,9 @@ int git_reflog_append(git_reflog *reflog, const git_oid *new_oid, assert(reflog && new_oid && committer); + if (!git_signature__has_valid_version(committer)) + return -1; + if (reflog_entry_new(&entry) < 0) return -1; diff --git a/src/signature.c b/src/signature.c index 0159488a4..008b13120 100644 --- a/src/signature.c +++ b/src/signature.c @@ -90,6 +90,7 @@ int git_signature_new(git_signature **sig_out, const char *name, const char *ema p = git__calloc(1, sizeof(git_signature)); GITERR_CHECK_ALLOC(p); + p->version = GIT_SIGNATURE_VERSION; if (process_trimming(name, &p->name, name + strlen(name), 1) < 0 || process_trimming(email, &p->email, email + strlen(email), 1) < 0) @@ -263,8 +264,9 @@ int git_signature__parse(git_signature *sig, const char **buffer_out, const char *buffer = *buffer_out; const char *line_end, *name_end, *email_end, *tz_start, *time_start; int error = 0; + git_signature initsig = GIT_SIGNATURE_INIT; - memset(sig, 0x0, sizeof(git_signature)); + memmove(sig, &initsig, sizeof(git_signature)); if ((line_end = memchr(buffer, ender, buffer_end - buffer)) == NULL) return signature_error("no newline given"); diff --git a/src/signature.h b/src/signature.h index 97b3a055e..599e19901 100644 --- a/src/signature.h +++ b/src/signature.h @@ -15,4 +15,16 @@ int git_signature__parse(git_signature *sig, const char **buffer_out, const char *buffer_end, const char *header, char ender); void git_signature__writebuf(git_buf *buf, const char *header, const git_signature *sig); +GIT_INLINE(bool) git_signature__has_valid_version(const git_signature *sig) +{ + if (!sig) + return true; + + if (sig->version > 0 && sig->version <= GIT_SIGNATURE_VERSION) + return true; + + giterr_set(GITERR_INVALID, "Invalid version %d on git_signature", sig->version); + return false; +} + #endif diff --git a/src/stash.c b/src/stash.c index 98c7a7c93..67fa49838 100644 --- a/src/stash.c +++ b/src/stash.c @@ -14,6 +14,7 @@ #include "git2/stash.h" #include "git2/status.h" #include "git2/checkout.h" +#include "signature.h" static int create_error(int error, const char *msg) { @@ -522,6 +523,9 @@ int git_stash_save( assert(out && repo && stasher); + if (!git_signature__has_valid_version(stasher)) + return -1; + if ((error = ensure_non_bare_repository(repo)) < 0) return error; diff --git a/src/tag.c b/src/tag.c index 606afd657..fb70837a2 100644 --- a/src/tag.c +++ b/src/tag.c @@ -244,6 +244,9 @@ static int git_tag_create__internal( assert(repo && tag_name && target); assert(!create_tag_annotation || (tagger && message)); + if (!git_signature__has_valid_version(tagger)) + return -1; + if (git_object_owner(target) != repo) { giterr_set(GITERR_INVALID, "The given target does not belong to this repository"); return -1; From c7231c45fecf6c0ae91815a82db7e98c94689497 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Fri, 30 Nov 2012 16:31:42 -0800 Subject: [PATCH 13/18] Deploy GITERR_CHECK_VERSION --- src/checkout.c | 20 ++------------------ src/commit.c | 5 ++--- src/common.h | 17 +++++++++++++++++ src/config.c | 15 +-------------- src/diff.c | 5 ++--- src/diff.h | 13 ------------- src/diff_output.c | 3 +-- src/diff_tform.c | 15 +-------------- src/notes.c | 10 ++++------ src/odb.c | 15 +-------------- src/reflog.c | 3 +-- src/remote.c | 30 ++---------------------------- src/repository.c | 15 +-------------- src/signature.h | 12 ------------ src/stash.c | 3 +-- src/status.c | 15 +-------------- src/tag.c | 3 +-- 17 files changed, 38 insertions(+), 161 deletions(-) diff --git a/src/checkout.c b/src/checkout.c index 06407bb30..640b04857 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -603,19 +603,6 @@ static int checkout_create_submodules( return 0; } -static bool opts_is_valid_version(git_checkout_opts *opts) -{ - if (!opts) - return true; - - if (opts->version > 0 && opts->version <= GIT_CHECKOUT_OPTS_VERSION) - return true; - - giterr_set(GITERR_INVALID, "Invalid version %d on git_checkout_opts structure", - opts->version); - return false; -} - int git_checkout_index( git_repository *repo, git_index *index, @@ -632,6 +619,8 @@ int git_checkout_index( assert(repo); + GITERR_CHECK_VERSION(opts, GIT_CHECKOUT_OPTS_VERSION, "git_checkout_opts"); + if ((error = git_repository__ensure_not_bare(repo, "checkout")) < 0) return error; @@ -639,11 +628,6 @@ int git_checkout_index( GIT_DIFF_INCLUDE_UNMODIFIED | GIT_DIFF_INCLUDE_UNTRACKED | GIT_DIFF_INCLUDE_TYPECHANGE | GIT_DIFF_SKIP_BINARY_CHECK; - if (!opts_is_valid_version(opts)) { - error = -1; - goto cleanup; - } - if (opts && opts->paths.count > 0) diff_opts.pathspec = opts->paths; diff --git a/src/commit.c b/src/commit.c index 5197fdb6d..f9606dd72 100644 --- a/src/commit.c +++ b/src/commit.c @@ -93,9 +93,8 @@ int git_commit_create( git_odb *odb; assert(git_object_owner((const git_object *)tree) == repo); - if (!git_signature__has_valid_version(author) || - !git_signature__has_valid_version(committer)) - return -1; + GITERR_CHECK_VERSION(author, GIT_SIGNATURE_VERSION, "git_signature"); + GITERR_CHECK_VERSION(committer, GIT_SIGNATURE_VERSION, "git_signature"); git_oid__writebuf(&commit, "tree ", git_object_id((const git_object *)tree)); diff --git a/src/common.h b/src/common.h index a35239e3d..b779d2800 100644 --- a/src/common.h +++ b/src/common.h @@ -65,6 +65,23 @@ void giterr_set(int error_class, const char *string, ...); */ void giterr_set_regex(const regex_t *regex, int error_code); +/** + * Check a versioned structure for validity + */ +GIT_INLINE(bool) giterr__check_version(const void *structure, unsigned int expected_max, const char *name) +{ + if (!structure) + return true; + + unsigned int actual = *(unsigned int*)structure; + if (actual > 0 && actual <= expected_max) + return true; + + giterr_set(GITERR_INVALID, "Invalid version %d on %s", actual, name); + return false; +} +#define GITERR_CHECK_VERSION(S,V,N) if (!giterr__check_version(S,V,N)) return -1 + /* NOTE: other giterr functions are in the public errors.h header file */ #include "util.h" diff --git a/src/config.c b/src/config.c index 913108abb..d422447cf 100644 --- a/src/config.c +++ b/src/config.c @@ -248,18 +248,6 @@ int git_config_open_level( return 0; } -static bool config_backend_has_valid_version(git_config_backend *backend) -{ - if (!backend) - return true; - - if (backend->version > 0 && backend->version <= GIT_CONFIG_BACKEND_VERSION) - return true; - - giterr_set(GITERR_INVALID, "Invalid version %d for git_config_backend", backend->version); - return false; -} - int git_config_add_backend( git_config *cfg, git_config_backend *file, @@ -271,8 +259,7 @@ int git_config_add_backend( assert(cfg && file); - if (!config_backend_has_valid_version(file)) - return -1; + GITERR_CHECK_VERSION(file, GIT_CONFIG_BACKEND_VERSION, "git_config_backend"); if ((result = file->open(file, level)) < 0) return result; diff --git a/src/diff.c b/src/diff.c index 722acbead..46d96bb96 100644 --- a/src/diff.c +++ b/src/diff.c @@ -758,9 +758,8 @@ fail: #define DIFF_FROM_ITERATORS(MAKE_FIRST, MAKE_SECOND) do { \ git_iterator *a = NULL, *b = NULL; \ char *pfx = opts ? git_pathspec_prefix(&opts->pathspec) : NULL; \ - if (!git_diff__opts_has_valid_version(opts)) \ - error = -1; \ - else if (!(error = MAKE_FIRST) && !(error = MAKE_SECOND)) \ + GITERR_CHECK_VERSION(opts, GIT_DIFF_OPTIONS_VERSION, "git_diff_options"); \ + if (!(error = MAKE_FIRST) && !(error = MAKE_SECOND)) \ error = diff_from_iterators(diff, repo, a, b, opts); \ git__free(pfx); git_iterator_free(a); git_iterator_free(b); \ } while (0) diff --git a/src/diff.h b/src/diff.h index 1f45af1cd..f93bab18d 100644 --- a/src/diff.h +++ b/src/diff.h @@ -62,18 +62,5 @@ extern int git_diff__oid_for_file( git_repository *, const char *, uint16_t, git_off_t, git_oid *); -GIT_INLINE(bool) git_diff__opts_has_valid_version(const git_diff_options *opts) -{ - if (!opts) - return true; - - if (opts->version > 0 && opts->version <= GIT_DIFF_OPTIONS_VERSION) - return true; - - giterr_set(GITERR_INVALID, "Invalid version %d in git_diff_options", opts->version); - return false; -} - - #endif diff --git a/src/diff_output.c b/src/diff_output.c index 6316bfa1e..b18255d58 100644 --- a/src/diff_output.c +++ b/src/diff_output.c @@ -1266,8 +1266,7 @@ int git_diff_blobs( git_diff_delta delta; git_diff_patch patch; - if (!git_diff__opts_has_valid_version(options)) - return -1; + GITERR_CHECK_VERSION(options, GIT_DIFF_OPTIONS_VERSION, "git_diff_options"); if (options && (options->flags & GIT_DIFF_REVERSE)) { git_blob *swap = old_blob; diff --git a/src/diff_tform.c b/src/diff_tform.c index 1df041044..0c588594a 100644 --- a/src/diff_tform.c +++ b/src/diff_tform.c @@ -168,18 +168,6 @@ int git_diff_merge( return error; } -static bool find_opts_has_valid_version(const git_diff_find_options *opts) -{ - if (!opts) - return true; - - if (opts->version > 0 && opts->version <= GIT_DIFF_FIND_OPTIONS_VERSION) - return true; - - giterr_set(GITERR_INVALID, "Invalid version %d on git_diff_find_options", opts->version); - return false; -} - #define DEFAULT_THRESHOLD 50 #define DEFAULT_BREAK_REWRITE_THRESHOLD 60 #define DEFAULT_TARGET_LIMIT 200 @@ -211,8 +199,7 @@ static int normalize_find_opts( opts->flags = GIT_DIFF_FIND_RENAMES | GIT_DIFF_FIND_COPIES; } - if (!find_opts_has_valid_version(opts)) - return -1; + GITERR_CHECK_VERSION(opts, GIT_DIFF_FIND_OPTIONS_VERSION, "git_diff_find_options"); /* some flags imply others */ diff --git a/src/notes.c b/src/notes.c index 87ee1e742..71a9e33ad 100644 --- a/src/notes.c +++ b/src/notes.c @@ -456,9 +456,8 @@ int git_note_create( git_commit *commit = NULL; git_tree *tree = NULL; - if (!git_signature__has_valid_version(author) || - !git_signature__has_valid_version(committer)) - return -1; + GITERR_CHECK_VERSION(author, GIT_SIGNATURE_VERSION, "git_signature"); + GITERR_CHECK_VERSION(committer, GIT_SIGNATURE_VERSION, "git_signature"); target = git_oid_allocfmt(oid); GITERR_CHECK_ALLOC(target); @@ -487,9 +486,8 @@ int git_note_remove(git_repository *repo, const char *notes_ref, git_commit *commit = NULL; git_tree *tree = NULL; - if (!git_signature__has_valid_version(author) || - !git_signature__has_valid_version(committer)) - return -1; + GITERR_CHECK_VERSION(author, GIT_SIGNATURE_VERSION, "git_signature"); + GITERR_CHECK_VERSION(committer, GIT_SIGNATURE_VERSION, "git_signature"); target = git_oid_allocfmt(oid); GITERR_CHECK_ALLOC(target); diff --git a/src/odb.c b/src/odb.c index e35e4caca..b6d1f798d 100644 --- a/src/odb.c +++ b/src/odb.c @@ -363,26 +363,13 @@ int git_odb_new(git_odb **out) return 0; } -static bool backend_has_valid_version(git_odb_backend *backend) -{ - if (!backend) - return true; - - if (backend->version > 0 && backend->version <= GIT_ODB_BACKEND_VERSION) - return true; - - giterr_set(GITERR_INVALID, "Invalid version %d on git_odb_backend", backend->version); - return false; -} - static int add_backend_internal(git_odb *odb, git_odb_backend *backend, int priority, int is_alternate) { backend_internal *internal; assert(odb && backend); - if (!backend_has_valid_version(backend)) - return -1; + GITERR_CHECK_VERSION(backend, GIT_ODB_BACKEND_VERSION, "git_odb_backend"); /* Check if the backend is already owned by another ODB */ assert(!backend->odb || backend->odb == odb); diff --git a/src/reflog.c b/src/reflog.c index 4d119bae7..c0af60f49 100644 --- a/src/reflog.c +++ b/src/reflog.c @@ -298,8 +298,7 @@ int git_reflog_append(git_reflog *reflog, const git_oid *new_oid, assert(reflog && new_oid && committer); - if (!git_signature__has_valid_version(committer)) - return -1; + GITERR_CHECK_VERSION(committer, GIT_SIGNATURE_VERSION, "git_signature"); if (reflog_entry_new(&entry) < 0) return -1; diff --git a/src/remote.c b/src/remote.c index 5fc6b4f35..4f5aa9dee 100644 --- a/src/remote.c +++ b/src/remote.c @@ -985,24 +985,11 @@ void git_remote_check_cert(git_remote *remote, int check) remote->check_cert = check; } -static bool callbacks_have_valid_version(git_remote_callbacks *callbacks) -{ - if (!callbacks) - return true; - - if (callbacks->version > 0 && callbacks->version <= GIT_REMOTE_CALLBACKS_VERSION) - return true; - - giterr_set(GITERR_INVALID, "Invalid version %d for git_remote_callbacks", callbacks->version); - return false; -} - int git_remote_set_callbacks(git_remote *remote, git_remote_callbacks *callbacks) { assert(remote && callbacks); - if (!callbacks_have_valid_version(callbacks)) - return -1; + GITERR_CHECK_VERSION(callbacks, GIT_REMOTE_CALLBACKS_VERSION, "git_remote_callbacks"); memcpy(&remote->callbacks, callbacks, sizeof(git_remote_callbacks)); @@ -1024,24 +1011,11 @@ void git_remote_set_cred_acquire_cb( remote->cred_acquire_cb = cred_acquire_cb; } -static bool transport_has_valid_version(const git_transport *transport) -{ - if (!transport) - return true; - - if (transport->version > 0 && transport->version <= GIT_TRANSPORT_VERSION) - return true; - - giterr_set(GITERR_INVALID, "Invalid version %d on git_transport", transport->version); - return false; -} - int git_remote_set_transport(git_remote *remote, git_transport *transport) { assert(remote && transport); - if (!transport_has_valid_version(transport)) - return -1; + GITERR_CHECK_VERSION(transport, GIT_TRANSPORT_VERSION, "git_transport"); if (remote->transport) { giterr_set(GITERR_NET, "A transport is already bound to this remote"); diff --git a/src/repository.c b/src/repository.c index 073efa484..10ed12b64 100644 --- a/src/repository.c +++ b/src/repository.c @@ -1160,18 +1160,6 @@ int git_repository_init( return git_repository_init_ext(repo_out, path, &opts); } -static bool options_have_valid_version(git_repository_init_options *opts) -{ - if (!opts) - return true; - - if (opts->version > 0 && opts->version <= GIT_REMOTE_CALLBACKS_VERSION) - return true; - - giterr_set(GITERR_INVALID, "Invalid version %d for git_repository_init_options", opts->version); - return false; -} - int git_repository_init_ext( git_repository **out, const char *given_repo, @@ -1182,8 +1170,7 @@ int git_repository_init_ext( assert(out && given_repo && opts); - if (!options_have_valid_version(opts)) - return -1; + GITERR_CHECK_VERSION(opts, GIT_REPOSITORY_INIT_OPTIONS_VERSION, "git_repository_init_options"); error = repo_init_directories(&repo_path, &wd_path, given_repo, opts); if (error < 0) diff --git a/src/signature.h b/src/signature.h index 599e19901..97b3a055e 100644 --- a/src/signature.h +++ b/src/signature.h @@ -15,16 +15,4 @@ int git_signature__parse(git_signature *sig, const char **buffer_out, const char *buffer_end, const char *header, char ender); void git_signature__writebuf(git_buf *buf, const char *header, const git_signature *sig); -GIT_INLINE(bool) git_signature__has_valid_version(const git_signature *sig) -{ - if (!sig) - return true; - - if (sig->version > 0 && sig->version <= GIT_SIGNATURE_VERSION) - return true; - - giterr_set(GITERR_INVALID, "Invalid version %d on git_signature", sig->version); - return false; -} - #endif diff --git a/src/stash.c b/src/stash.c index 67fa49838..14b48a595 100644 --- a/src/stash.c +++ b/src/stash.c @@ -523,8 +523,7 @@ int git_stash_save( assert(out && repo && stasher); - if (!git_signature__has_valid_version(stasher)) - return -1; + GITERR_CHECK_VERSION(stasher, GIT_SIGNATURE_VERSION, "git_signature"); if ((error = ensure_non_bare_repository(repo)) < 0) return error; diff --git a/src/status.c b/src/status.c index f23f40e6d..1ad835adb 100644 --- a/src/status.c +++ b/src/status.c @@ -101,18 +101,6 @@ static int status_invoke_cb( return usercb->cb(path, status, usercb->payload); } -static bool options_have_valid_version(const git_status_options *opts) -{ - if (!opts) - return true; - - if (opts->version > 0 && opts->version <= GIT_REMOTE_CALLBACKS_VERSION) - return true; - - giterr_set(GITERR_INVALID, "Invalid version %d for git_repository_init_options", opts->version); - return false; -} - int git_status_foreach_ext( git_repository *repo, const git_status_options *opts, @@ -129,8 +117,7 @@ int git_status_foreach_ext( assert(show <= GIT_STATUS_SHOW_INDEX_THEN_WORKDIR); - if (!options_have_valid_version(opts)) - return -1; + GITERR_CHECK_VERSION(opts, GIT_STATUS_OPTIONS_VERSION, "git_status_options"); if (show != GIT_STATUS_SHOW_INDEX_ONLY && (err = git_repository__ensure_not_bare(repo, "status")) < 0) diff --git a/src/tag.c b/src/tag.c index fb70837a2..c3b3319fb 100644 --- a/src/tag.c +++ b/src/tag.c @@ -244,8 +244,7 @@ static int git_tag_create__internal( assert(repo && tag_name && target); assert(!create_tag_annotation || (tagger && message)); - if (!git_signature__has_valid_version(tagger)) - return -1; + GITERR_CHECK_VERSION(tagger, GIT_SIGNATURE_VERSION, "git_signature"); if (git_object_owner(target) != repo) { giterr_set(GITERR_INVALID, "The given target does not belong to this repository"); From 0ab3a2ab2c39a99b7cb3c969fd7b896afcec4885 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Fri, 30 Nov 2012 20:34:50 -0800 Subject: [PATCH 14/18] Deploy GIT_INIT_STRUCTURE --- src/checkout.c | 4 +--- src/common.h | 10 ++++++++++ tests-clar/checkout/checkout_util.h | 5 ----- tests-clar/checkout/index.c | 5 ++--- tests-clar/checkout/tree.c | 3 +-- tests-clar/diff/blob.c | 2 +- tests-clar/diff/diff_helpers.h | 6 ------ 7 files changed, 15 insertions(+), 20 deletions(-) delete mode 100644 tests-clar/checkout/checkout_util.h diff --git a/src/checkout.c b/src/checkout.c index 640b04857..ae4066243 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -225,12 +225,10 @@ static int retrieve_symlink_caps(git_repository *repo, bool *can_symlink) static void normalize_options( git_checkout_opts *normalized, git_checkout_opts *proposed) { - git_checkout_opts init_opts = GIT_CHECKOUT_OPTS_INIT; - assert(normalized); if (!proposed) - memmove(normalized, &init_opts, sizeof(git_checkout_opts)); + GIT_INIT_STRUCTURE(normalized, GIT_CHECKOUT_OPTS_VERSION); else memmove(normalized, proposed, sizeof(git_checkout_opts)); diff --git a/src/common.h b/src/common.h index b779d2800..3e7b0658f 100644 --- a/src/common.h +++ b/src/common.h @@ -82,6 +82,16 @@ GIT_INLINE(bool) giterr__check_version(const void *structure, unsigned int expec } #define GITERR_CHECK_VERSION(S,V,N) if (!giterr__check_version(S,V,N)) return -1 +/** + * Initialize a structure with a version. + */ +GIT_INLINE(void) git__init_structure(void *structure, size_t len, unsigned int version) +{ + memset(structure, 0, len); + *((int*)structure) = version; +} +#define GIT_INIT_STRUCTURE(S,V) git__init_structure(S, sizeof(*S), V) + /* NOTE: other giterr functions are in the public errors.h header file */ #include "util.h" diff --git a/tests-clar/checkout/checkout_util.h b/tests-clar/checkout/checkout_util.h deleted file mode 100644 index d6aa7ed47..000000000 --- a/tests-clar/checkout/checkout_util.h +++ /dev/null @@ -1,5 +0,0 @@ -GIT_INLINE(void) reset_checkout_opts(git_checkout_opts *opts) -{ - git_checkout_opts init_opts = GIT_CHECKOUT_OPTS_INIT; - memmove(opts, &init_opts, sizeof(git_checkout_opts)); -} diff --git a/tests-clar/checkout/index.c b/tests-clar/checkout/index.c index e86dfe954..a67765b26 100644 --- a/tests-clar/checkout/index.c +++ b/tests-clar/checkout/index.c @@ -2,7 +2,6 @@ #include "git2/checkout.h" #include "repository.h" -#include "checkout_util.h" static git_repository *g_repo; static git_checkout_opts g_opts; @@ -26,7 +25,7 @@ void test_checkout_index__initialize(void) { git_tree *tree; - reset_checkout_opts(&g_opts); + GIT_INIT_STRUCTURE(&g_opts, GIT_CHECKOUT_OPTS_VERSION); g_opts.checkout_strategy = GIT_CHECKOUT_SAFE; g_repo = cl_git_sandbox_init("testrepo"); @@ -67,7 +66,7 @@ void test_checkout_index__cannot_checkout_a_bare_repository(void) { test_checkout_index__cleanup(); - reset_checkout_opts(&g_opts); + GIT_INIT_STRUCTURE(&g_opts, GIT_CHECKOUT_OPTS_VERSION); g_repo = cl_git_sandbox_init("testrepo.git"); cl_git_fail(git_checkout_index(g_repo, NULL, NULL)); diff --git a/tests-clar/checkout/tree.c b/tests-clar/checkout/tree.c index fe5bd4ff8..88dbe4ffc 100644 --- a/tests-clar/checkout/tree.c +++ b/tests-clar/checkout/tree.c @@ -2,7 +2,6 @@ #include "git2/checkout.h" #include "repository.h" -#include "checkout_util.h" static git_repository *g_repo; static git_checkout_opts g_opts; @@ -12,7 +11,7 @@ void test_checkout_tree__initialize(void) { g_repo = cl_git_sandbox_init("testrepo"); - reset_checkout_opts(&g_opts); + GIT_INIT_STRUCTURE(&g_opts, GIT_CHECKOUT_OPTS_VERSION); g_opts.checkout_strategy = GIT_CHECKOUT_SAFE; } diff --git a/tests-clar/diff/blob.c b/tests-clar/diff/blob.c index ae3129c9f..d7fdba0e6 100644 --- a/tests-clar/diff/blob.c +++ b/tests-clar/diff/blob.c @@ -12,7 +12,7 @@ void test_diff_blob__initialize(void) g_repo = cl_git_sandbox_init("attr"); - reset_diff_opts(&opts); + GIT_INIT_STRUCTURE(&opts, GIT_DIFF_OPTIONS_VERSION); opts.context_lines = 1; opts.interhunk_lines = 0; diff --git a/tests-clar/diff/diff_helpers.h b/tests-clar/diff/diff_helpers.h index 0f7c0e4f8..49c265285 100644 --- a/tests-clar/diff/diff_helpers.h +++ b/tests-clar/diff/diff_helpers.h @@ -49,9 +49,3 @@ extern int diff_foreach_via_iterator( extern void diff_print(FILE *fp, git_diff_list *diff); - -GIT_INLINE(void) reset_diff_opts(git_diff_options *opts) -{ - git_diff_options init = GIT_DIFF_OPTIONS_INIT; - memmove(opts, &init, sizeof(init)); -} From 7bcfbe16c59d6f3a47cfbfa65952623081c8f7de Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Fri, 30 Nov 2012 20:35:01 -0800 Subject: [PATCH 15/18] Make constant name all-caps --- include/git2/diff.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/git2/diff.h b/include/git2/diff.h index 16a24b5f6..6732c30da 100644 --- a/include/git2/diff.h +++ b/include/git2/diff.h @@ -107,7 +107,7 @@ typedef enum { */ typedef struct { unsigned int version; /**< version for the struct */ - uint32_t flags; /**< defaults to git_diff_normal */ + uint32_t flags; /**< defaults to GIT_DIFF_NORMAL */ uint16_t context_lines; /**< defaults to 3 */ uint16_t interhunk_lines; /**< defaults to 0 */ char *old_prefix; /**< defaults to "a" */ From 2da619abdebe78ce90be5e7d58e2257ec1777003 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Mon, 3 Dec 2012 12:41:38 -0800 Subject: [PATCH 16/18] Remove GIT_CRED_VERSION and friends --- include/git2/transport.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/include/git2/transport.h b/include/git2/transport.h index 84d71c612..60ac3f242 100644 --- a/include/git2/transport.h +++ b/include/git2/transport.h @@ -31,15 +31,11 @@ typedef enum { /* The base structure for all credential types */ typedef struct git_cred { - unsigned int version; /* This should update if subtypes are extended */ git_credtype_t credtype; void (*free)( struct git_cred *cred); } git_cred; -#define GIT_CRED_VERSION 1 -#define GIT_CRED_INIT {GIT_CRED_VERSION, 0} - /* A plaintext username and password */ typedef struct git_cred_userpass_plaintext { git_cred parent; From de70aea6b1c09b67f233ef0bee83864ce90260b2 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Mon, 3 Dec 2012 12:41:50 -0800 Subject: [PATCH 17/18] Remove GIT_SIGNATURE_VERSION and friends --- include/git2/types.h | 4 ---- src/commit.c | 2 -- src/notes.c | 6 ------ src/reflog.c | 3 --- src/signature.c | 4 +--- src/stash.c | 2 -- src/tag.c | 2 -- tests-clar/commit/parse.c | 4 ++-- 8 files changed, 3 insertions(+), 24 deletions(-) diff --git a/include/git2/types.h b/include/git2/types.h index d83b1d14b..06fcf3613 100644 --- a/include/git2/types.h +++ b/include/git2/types.h @@ -151,15 +151,11 @@ typedef struct git_time { /** An action signature (e.g. for committers, taggers, etc) */ typedef struct git_signature { - unsigned int version; char *name; /** full name of the author */ char *email; /** email of the author */ git_time when; /** time when the action happened */ } git_signature; -#define GIT_SIGNATURE_VERSION 1 -#define GIT_SIGNATURE_INIT {GIT_SIGNATURE_VERSION, 0} - /** In-memory representation of a reference. */ typedef struct git_reference git_reference; diff --git a/src/commit.c b/src/commit.c index f9606dd72..4072518ff 100644 --- a/src/commit.c +++ b/src/commit.c @@ -93,8 +93,6 @@ int git_commit_create( git_odb *odb; assert(git_object_owner((const git_object *)tree) == repo); - GITERR_CHECK_VERSION(author, GIT_SIGNATURE_VERSION, "git_signature"); - GITERR_CHECK_VERSION(committer, GIT_SIGNATURE_VERSION, "git_signature"); git_oid__writebuf(&commit, "tree ", git_object_id((const git_object *)tree)); diff --git a/src/notes.c b/src/notes.c index 71a9e33ad..f96b5b139 100644 --- a/src/notes.c +++ b/src/notes.c @@ -456,9 +456,6 @@ int git_note_create( git_commit *commit = NULL; git_tree *tree = NULL; - GITERR_CHECK_VERSION(author, GIT_SIGNATURE_VERSION, "git_signature"); - GITERR_CHECK_VERSION(committer, GIT_SIGNATURE_VERSION, "git_signature"); - target = git_oid_allocfmt(oid); GITERR_CHECK_ALLOC(target); @@ -486,9 +483,6 @@ int git_note_remove(git_repository *repo, const char *notes_ref, git_commit *commit = NULL; git_tree *tree = NULL; - GITERR_CHECK_VERSION(author, GIT_SIGNATURE_VERSION, "git_signature"); - GITERR_CHECK_VERSION(committer, GIT_SIGNATURE_VERSION, "git_signature"); - target = git_oid_allocfmt(oid); GITERR_CHECK_ALLOC(target); diff --git a/src/reflog.c b/src/reflog.c index c0af60f49..ac481fb81 100644 --- a/src/reflog.c +++ b/src/reflog.c @@ -112,7 +112,6 @@ static int reflog_parse(git_reflog *log, const char *buf, size_t buf_size) entry->committer = git__malloc(sizeof(git_signature)); GITERR_CHECK_ALLOC(entry->committer); - entry->committer->version = GIT_SIGNATURE_VERSION; if (git_oid_fromstrn(&entry->oid_old, buf, GIT_OID_HEXSZ) < 0) goto fail; @@ -298,8 +297,6 @@ int git_reflog_append(git_reflog *reflog, const git_oid *new_oid, assert(reflog && new_oid && committer); - GITERR_CHECK_VERSION(committer, GIT_SIGNATURE_VERSION, "git_signature"); - if (reflog_entry_new(&entry) < 0) return -1; diff --git a/src/signature.c b/src/signature.c index 008b13120..7d043e6cf 100644 --- a/src/signature.c +++ b/src/signature.c @@ -90,7 +90,6 @@ int git_signature_new(git_signature **sig_out, const char *name, const char *ema p = git__calloc(1, sizeof(git_signature)); GITERR_CHECK_ALLOC(p); - p->version = GIT_SIGNATURE_VERSION; if (process_trimming(name, &p->name, name + strlen(name), 1) < 0 || process_trimming(email, &p->email, email + strlen(email), 1) < 0) @@ -264,9 +263,8 @@ int git_signature__parse(git_signature *sig, const char **buffer_out, const char *buffer = *buffer_out; const char *line_end, *name_end, *email_end, *tz_start, *time_start; int error = 0; - git_signature initsig = GIT_SIGNATURE_INIT; - memmove(sig, &initsig, sizeof(git_signature)); + memset(sig, 0, sizeof(git_signature)); if ((line_end = memchr(buffer, ender, buffer_end - buffer)) == NULL) return signature_error("no newline given"); diff --git a/src/stash.c b/src/stash.c index 14b48a595..e32d8fa31 100644 --- a/src/stash.c +++ b/src/stash.c @@ -523,8 +523,6 @@ int git_stash_save( assert(out && repo && stasher); - GITERR_CHECK_VERSION(stasher, GIT_SIGNATURE_VERSION, "git_signature"); - if ((error = ensure_non_bare_repository(repo)) < 0) return error; diff --git a/src/tag.c b/src/tag.c index c3b3319fb..606afd657 100644 --- a/src/tag.c +++ b/src/tag.c @@ -244,8 +244,6 @@ static int git_tag_create__internal( assert(repo && tag_name && target); assert(!create_tag_annotation || (tagger && message)); - GITERR_CHECK_VERSION(tagger, GIT_SIGNATURE_VERSION, "git_signature"); - if (git_object_owner(target) != repo) { giterr_set(GITERR_INVALID, "The given target does not belong to this repository"); return -1; diff --git a/tests-clar/commit/parse.c b/tests-clar/commit/parse.c index 37e38db53..8075f2619 100644 --- a/tests-clar/commit/parse.c +++ b/tests-clar/commit/parse.c @@ -149,7 +149,7 @@ void test_commit_parse__signature(void) { const char *str = passcase->string; size_t len = strlen(passcase->string); - struct git_signature person = GIT_SIGNATURE_INIT; + struct git_signature person = {0}; cl_git_pass(git_signature__parse(&person, &str, str + len, passcase->header, '\n')); cl_assert(strcmp(passcase->name, person.name) == 0); cl_assert(strcmp(passcase->email, person.email) == 0); @@ -162,7 +162,7 @@ void test_commit_parse__signature(void) { const char *str = failcase->string; size_t len = strlen(failcase->string); - git_signature person = GIT_SIGNATURE_INIT; + git_signature person = {0}; cl_git_fail(git_signature__parse(&person, &str, str + len, failcase->header, '\n')); git__free(person.name); git__free(person.email); } From ee1c33b146a366260a4648b1f29f470fedaca0fa Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Mon, 3 Dec 2012 12:45:15 -0800 Subject: [PATCH 18/18] Don't unconstify when casting --- src/common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common.h b/src/common.h index 3e7b0658f..08228c2a9 100644 --- a/src/common.h +++ b/src/common.h @@ -73,7 +73,7 @@ GIT_INLINE(bool) giterr__check_version(const void *structure, unsigned int expec if (!structure) return true; - unsigned int actual = *(unsigned int*)structure; + unsigned int actual = *(const unsigned int*)structure; if (actual > 0 && actual <= expected_max) return true;