From 5572d2b8a1668f3c2f35ec91d3f864f28f8ef987 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 28 Jan 2014 11:44:17 -0800 Subject: [PATCH 1/7] Some missing oid to id renames --- include/git2/blob.h | 5 +++-- src/blob.c | 41 ++++++++++++++++++++++------------------- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/include/git2/blob.h b/include/git2/blob.h index 6ba5e9f9c..ac4d84392 100644 --- a/include/git2/blob.h +++ b/include/git2/blob.h @@ -196,13 +196,14 @@ GIT_EXTERN(int) git_blob_create_fromchunks( /** * Write an in-memory buffer to the ODB as a blob * - * @param oid return the oid of the written blob + * @param id return the id of the written blob * @param repo repository where to blob will be written * @param buffer data to be written into the blob * @param len length of the data * @return 0 or an error code */ -GIT_EXTERN(int) git_blob_create_frombuffer(git_oid *oid, git_repository *repo, const void *buffer, size_t len); +GIT_EXTERN(int) git_blob_create_frombuffer( + git_oid *id, git_repository *repo, const void *buffer, size_t len); /** * Determine if the blob content is most certainly binary or not. diff --git a/src/blob.c b/src/blob.c index 2e924f37f..faf8a4a99 100644 --- a/src/blob.c +++ b/src/blob.c @@ -50,25 +50,28 @@ int git_blob__parse(void *blob, git_odb_object *odb_obj) return 0; } -int git_blob_create_frombuffer(git_oid *oid, git_repository *repo, const void *buffer, size_t len) +int git_blob_create_frombuffer( + git_oid *id, git_repository *repo, const void *buffer, size_t len) { int error; git_odb *odb; git_odb_stream *stream; + assert(id && repo); + if ((error = git_repository_odb__weakptr(&odb, repo)) < 0 || (error = git_odb_open_wstream(&stream, odb, len, GIT_OBJ_BLOB)) < 0) return error; if ((error = git_odb_stream_write(stream, buffer, len)) == 0) - error = git_odb_stream_finalize_write(oid, stream); + error = git_odb_stream_finalize_write(id, stream); git_odb_stream_free(stream); return error; } static int write_file_stream( - git_oid *oid, git_odb *odb, const char *path, git_off_t file_size) + git_oid *id, git_odb *odb, const char *path, git_off_t file_size) { int fd, error; char buffer[4096]; @@ -97,14 +100,14 @@ static int write_file_stream( } if (!error) - error = git_odb_stream_finalize_write(oid, stream); + error = git_odb_stream_finalize_write(id, stream); git_odb_stream_free(stream); return error; } static int write_file_filtered( - git_oid *oid, + git_oid *id, git_off_t *size, git_odb *odb, const char *full_path, @@ -119,7 +122,7 @@ static int write_file_filtered( if (!error) { *size = tgt.size; - error = git_odb_write(oid, odb, tgt.ptr, tgt.size, GIT_OBJ_BLOB); + error = git_odb_write(id, odb, tgt.ptr, tgt.size, GIT_OBJ_BLOB); } git_buf_free(&tgt); @@ -127,7 +130,7 @@ static int write_file_filtered( } static int write_symlink( - git_oid *oid, git_odb *odb, const char *path, size_t link_size) + git_oid *id, git_odb *odb, const char *path, size_t link_size) { char *link_data; ssize_t read_len; @@ -143,13 +146,13 @@ static int write_symlink( return -1; } - error = git_odb_write(oid, odb, (void *)link_data, link_size, GIT_OBJ_BLOB); + error = git_odb_write(id, odb, (void *)link_data, link_size, GIT_OBJ_BLOB); git__free(link_data); return error; } int git_blob__create_from_paths( - git_oid *oid, + git_oid *id, struct stat *out_st, git_repository *repo, const char *content_path, @@ -188,7 +191,7 @@ int git_blob__create_from_paths( mode = hint_mode ? hint_mode : st.st_mode; if (S_ISLNK(mode)) { - error = write_symlink(oid, odb, content_path, (size_t)size); + error = write_symlink(id, odb, content_path, (size_t)size); } else { git_filter_list *fl = NULL; @@ -202,10 +205,10 @@ int git_blob__create_from_paths( else if (fl == NULL) /* No filters need to be applied to the document: we can stream * directly from disk */ - error = write_file_stream(oid, odb, content_path, size); + error = write_file_stream(id, odb, content_path, size); else { /* We need to apply one or more filters */ - error = write_file_filtered(oid, &size, odb, content_path, fl); + error = write_file_filtered(id, &size, odb, content_path, fl); git_filter_list_free(fl); } @@ -233,13 +236,13 @@ done: } int git_blob_create_fromworkdir( - git_oid *oid, git_repository *repo, const char *path) + git_oid *id, git_repository *repo, const char *path) { - return git_blob__create_from_paths(oid, NULL, repo, NULL, path, 0, true); + return git_blob__create_from_paths(id, NULL, repo, NULL, path, 0, true); } int git_blob_create_fromdisk( - git_oid *oid, git_repository *repo, const char *path) + git_oid *id, git_repository *repo, const char *path) { int error; git_buf full_path = GIT_BUF_INIT; @@ -257,7 +260,7 @@ int git_blob_create_fromdisk( hintpath += strlen(workdir); error = git_blob__create_from_paths( - oid, NULL, repo, git_buf_cstr(&full_path), hintpath, 0, true); + id, NULL, repo, git_buf_cstr(&full_path), hintpath, 0, true); git_buf_free(&full_path); return error; @@ -266,7 +269,7 @@ int git_blob_create_fromdisk( #define BUFFER_SIZE 4096 int git_blob_create_fromchunks( - git_oid *oid, + git_oid *id, git_repository *repo, const char *hintpath, int (*source_cb)(char *content, size_t max_length, void *payload), @@ -277,7 +280,7 @@ int git_blob_create_fromchunks( git_filebuf file = GIT_FILEBUF_INIT; git_buf path = GIT_BUF_INIT; - assert(oid && repo && source_cb); + assert(id && repo && source_cb); if ((error = git_buf_joinpath( &path, git_repository_path(repo), GIT_OBJECTS_DIR "streamed")) < 0) @@ -313,7 +316,7 @@ int git_blob_create_fromchunks( goto cleanup; error = git_blob__create_from_paths( - oid, NULL, repo, file.path_lock, hintpath, 0, hintpath != NULL); + id, NULL, repo, file.path_lock, hintpath, 0, hintpath != NULL); cleanup: git_buf_free(&path); From c0644c3fbb31c381afb2d0658b5c6e83432fd8c9 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 28 Jan 2014 11:45:06 -0800 Subject: [PATCH 2/7] Make submodule fetchRecurse match other options This removes the fetchRecurse compiler warnings and makes the behavior match the other submodule options (i.e. the in-memory setting can be reset to the on-disk value). --- include/git2/submodule.h | 13 ------------- include/git2/types.h | 19 +++++++++++++++++++ src/submodule.c | 7 ++++++- src/submodule.h | 5 ++++- tests/submodule/modify.c | 34 ++++++++++++++++++++-------------- 5 files changed, 49 insertions(+), 29 deletions(-) diff --git a/include/git2/submodule.h b/include/git2/submodule.h index af08ba6eb..d72432610 100644 --- a/include/git2/submodule.h +++ b/include/git2/submodule.h @@ -105,19 +105,6 @@ typedef enum { GIT_SUBMODULE_STATUS_WD_WD_MODIFIED | \ GIT_SUBMODULE_STATUS_WD_UNTRACKED)) != 0) -/** - * Options for submodule recurse. - * - * * GIT_SUBMODULE_RECURSE_NO - do no recurse into submodules - * * GIT_SUBMODULE_RECURSE_YES - recurse into submodules - * * GIT_SUBMODULE_RECURSE_ONDEMAND - recurse into submodules only when commit not already in local clone - */ -typedef enum { - GIT_SUBMODULE_RECURSE_NO = 0, - GIT_SUBMODULE_RECURSE_YES = 1, - GIT_SUBMODULE_RECURSE_ONDEMAND = 2, -} git_submodule_recurse_t; - /** * Lookup submodule information by name or path. * diff --git a/include/git2/types.h b/include/git2/types.h index d88815d80..2229f6bf4 100644 --- a/include/git2/types.h +++ b/include/git2/types.h @@ -323,6 +323,25 @@ typedef enum { GIT_SUBMODULE_IGNORE_DEFAULT = 0 } git_submodule_ignore_t; +/** + * Options for submodule recurse. + * + * Represent the value of `submodule.$name.fetchRecurseSubmodules` + * + * * GIT_SUBMODULE_RECURSE_RESET - reset to the on-disk value + * * GIT_SUBMODULE_RECURSE_NO - do no recurse into submodules + * * GIT_SUBMODULE_RECURSE_YES - recurse into submodules + * * GIT_SUBMODULE_RECURSE_ONDEMAND - recurse into submodules only when + * commit not already in local clone + */ +typedef enum { + GIT_SUBMODULE_RECURSE_RESET = -1, + + GIT_SUBMODULE_RECURSE_NO = 0, + GIT_SUBMODULE_RECURSE_YES = 1, + GIT_SUBMODULE_RECURSE_ONDEMAND = 2, +} git_submodule_recurse_t; + /** @} */ GIT_END_DECL diff --git a/src/submodule.c b/src/submodule.c index 720681db9..2388bd144 100644 --- a/src/submodule.c +++ b/src/submodule.c @@ -498,6 +498,7 @@ int git_submodule_save(git_submodule *submodule) submodule->ignore_default = submodule->ignore; submodule->update_default = submodule->update; + submodule->fetch_recurse_default = submodule->fetch_recurse; submodule->flags |= GIT_SUBMODULE_STATUS_IN_CONFIG; cleanup: @@ -650,6 +651,9 @@ git_submodule_recurse_t git_submodule_set_fetch_recurse_submodules( assert(submodule); + if (fetch_recurse_submodules == GIT_SUBMODULE_RECURSE_RESET) + fetch_recurse_submodules = submodule->fetch_recurse_default; + old = submodule->fetch_recurse; submodule->fetch_recurse = fetch_recurse_submodules; return old; @@ -1000,7 +1004,7 @@ static git_submodule *submodule_alloc(git_repository *repo, const char *name) GIT_REFCOUNT_INC(sm); sm->ignore = sm->ignore_default = GIT_SUBMODULE_IGNORE_NONE; sm->update = sm->update_default = GIT_SUBMODULE_UPDATE_CHECKOUT; - sm->fetch_recurse = GIT_SUBMODULE_RECURSE_YES; + sm->fetch_recurse = sm->fetch_recurse_default = GIT_SUBMODULE_RECURSE_NO; sm->repo = repo; sm->branch = NULL; @@ -1218,6 +1222,7 @@ static int submodule_load_from_config( else if (strcasecmp(property, "fetchRecurseSubmodules") == 0) { if (git_submodule_parse_recurse(&sm->fetch_recurse, value) < 0) return -1; + sm->fetch_recurse_default = sm->fetch_recurse; } else if (strcasecmp(property, "ignore") == 0) { if ((error = git_submodule_parse_ignore(&sm->ignore, value)) < 0) diff --git a/src/submodule.h b/src/submodule.h index 94748aca0..5e532e1ae 100644 --- a/src/submodule.h +++ b/src/submodule.h @@ -60,7 +60,9 @@ * - `update_default` is the update value from the config * - `ignore` is a git_submodule_ignore_t value - see gitmodules(5) ignore. * - `ignore_default` is the ignore value from the config - * - `fetch_recurse` is 0 or 1 - see gitmodules(5) fetchRecurseSubmodules. + * - `fetch_recurse` is a git_submodule_recurse_t value - see gitmodules(5) + * fetchRecurseSubmodules. + * - `fetch_recurse_default` is the recurse value from the config * * - `repo` is the parent repository that contains this submodule. * - `flags` after for internal use, tracking where this submodule has been @@ -87,6 +89,7 @@ struct git_submodule { git_submodule_ignore_t ignore; git_submodule_ignore_t ignore_default; git_submodule_recurse_t fetch_recurse; + git_submodule_recurse_t fetch_recurse_default; /* internal information */ git_repository *repo; diff --git a/tests/submodule/modify.c b/tests/submodule/modify.c index 8ec9fceb0..e3e4d8aed 100644 --- a/tests/submodule/modify.c +++ b/tests/submodule/modify.c @@ -178,25 +178,28 @@ void test_submodule_modify__edit_and_save(void) cl_git_pass(git_submodule_set_url(sm1, SM_LIBGIT2_URL)); old_ignore = git_submodule_set_ignore(sm1, GIT_SUBMODULE_IGNORE_UNTRACKED); old_update = git_submodule_set_update(sm1, GIT_SUBMODULE_UPDATE_REBASE); - old_fetchrecurse = git_submodule_set_fetch_recurse_submodules(sm1, GIT_SUBMODULE_RECURSE_YES); + old_fetchrecurse = git_submodule_set_fetch_recurse_submodules( + sm1, GIT_SUBMODULE_RECURSE_YES); cl_assert_equal_s(SM_LIBGIT2_URL, git_submodule_url(sm1)); cl_assert_equal_i( - (int)GIT_SUBMODULE_IGNORE_UNTRACKED, (int)git_submodule_ignore(sm1)); + GIT_SUBMODULE_IGNORE_UNTRACKED, git_submodule_ignore(sm1)); cl_assert_equal_i( - (int)GIT_SUBMODULE_UPDATE_REBASE, (int)git_submodule_update(sm1)); - cl_assert_equal_i(GIT_SUBMODULE_RECURSE_YES, git_submodule_fetch_recurse_submodules(sm1)); + GIT_SUBMODULE_UPDATE_REBASE, git_submodule_update(sm1)); + cl_assert_equal_i( + GIT_SUBMODULE_RECURSE_YES, git_submodule_fetch_recurse_submodules(sm1)); /* revert without saving (and confirm setters return old value) */ cl_git_pass(git_submodule_set_url(sm1, old_url)); cl_assert_equal_i( - (int)GIT_SUBMODULE_IGNORE_UNTRACKED, - (int)git_submodule_set_ignore(sm1, GIT_SUBMODULE_IGNORE_RESET)); + GIT_SUBMODULE_IGNORE_UNTRACKED, + git_submodule_set_ignore(sm1, GIT_SUBMODULE_IGNORE_RESET)); cl_assert_equal_i( - (int)GIT_SUBMODULE_UPDATE_REBASE, - (int)git_submodule_set_update(sm1, GIT_SUBMODULE_UPDATE_RESET)); + GIT_SUBMODULE_UPDATE_REBASE, + git_submodule_set_update(sm1, GIT_SUBMODULE_UPDATE_RESET)); cl_assert_equal_i( - GIT_SUBMODULE_RECURSE_YES, git_submodule_set_fetch_recurse_submodules(sm1, old_fetchrecurse)); + GIT_SUBMODULE_RECURSE_YES, git_submodule_set_fetch_recurse_submodules( + sm1, GIT_SUBMODULE_RECURSE_RESET)); /* check that revert was successful */ cl_assert_equal_s(old_url, git_submodule_url(sm1)); @@ -243,19 +246,22 @@ void test_submodule_modify__edit_and_save(void) cl_assert_equal_s(SM_LIBGIT2_URL, git_submodule_url(sm2)); cl_assert_equal_i( - (int)GIT_SUBMODULE_IGNORE_UNTRACKED, (int)git_submodule_ignore(sm2)); + GIT_SUBMODULE_IGNORE_UNTRACKED, git_submodule_ignore(sm2)); cl_assert_equal_i( - (int)GIT_SUBMODULE_UPDATE_REBASE, (int)git_submodule_update(sm2)); - cl_assert_equal_i(GIT_SUBMODULE_RECURSE_YES, git_submodule_fetch_recurse_submodules(sm2)); + GIT_SUBMODULE_UPDATE_REBASE, git_submodule_update(sm2)); + cl_assert_equal_i( + GIT_SUBMODULE_RECURSE_NO, git_submodule_fetch_recurse_submodules(sm2)); /* set fetchRecurseSubmodules on-demand */ cl_git_pass(git_submodule_reload(sm1)); git_submodule_set_fetch_recurse_submodules(sm1, GIT_SUBMODULE_RECURSE_ONDEMAND); - cl_assert_equal_i(GIT_SUBMODULE_RECURSE_ONDEMAND, git_submodule_fetch_recurse_submodules(sm1)); + cl_assert_equal_i( + GIT_SUBMODULE_RECURSE_ONDEMAND, git_submodule_fetch_recurse_submodules(sm1)); /* call save */ cl_git_pass(git_submodule_save(sm1)); cl_git_pass(git_submodule_reload(sm1)); - cl_assert_equal_i(GIT_SUBMODULE_RECURSE_ONDEMAND, git_submodule_fetch_recurse_submodules(sm1)); + cl_assert_equal_i( + GIT_SUBMODULE_RECURSE_ONDEMAND, git_submodule_fetch_recurse_submodules(sm1)); git_repository_free(r2); git__free(old_url); From 3cf11eef17d8359c032844de945da6e983572ecc Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 28 Jan 2014 11:47:33 -0800 Subject: [PATCH 3/7] Misc cleanups --- src/attr.c | 9 +++++++++ src/branch.c | 39 ++++++++++++++++----------------------- tests/status/renames.c | 1 - 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/attr.c b/src/attr.c index ff4446e2f..15ed5c9e0 100644 --- a/src/attr.c +++ b/src/attr.c @@ -49,6 +49,8 @@ int git_attr_get( git_attr_name attr; git_attr_rule *rule; + assert(value && repo && name); + *value = NULL; if (git_attr_path__init(&path, pathname, git_repository_workdir(repo)) < 0) @@ -103,6 +105,11 @@ int git_attr_get_many( attr_get_many_info *info = NULL; size_t num_found = 0; + if (!num_attr) + return 0; + + assert(values && repo && names); + if (git_attr_path__init(&path, pathname, git_repository_workdir(repo)) < 0) return -1; @@ -169,6 +176,8 @@ int git_attr_foreach( git_attr_assignment *assign; git_strmap *seen = NULL; + assert(repo && callback); + if (git_attr_path__init(&path, pathname, git_repository_workdir(repo)) < 0) return -1; diff --git a/src/branch.c b/src/branch.c index a1a04b2b4..d0dc21b85 100644 --- a/src/branch.c +++ b/src/branch.c @@ -21,27 +21,22 @@ static int retrieve_branch_reference( const char *branch_name, int is_remote) { - git_reference *branch; - int error = -1; + git_reference *branch = NULL; + int error = 0; char *prefix; git_buf ref_name = GIT_BUF_INIT; - *branch_reference_out = NULL; - prefix = is_remote ? GIT_REFS_REMOTES_DIR : GIT_REFS_HEADS_DIR; - if (git_buf_joinpath(&ref_name, prefix, branch_name) < 0) - goto cleanup; + if ((error = git_buf_joinpath(&ref_name, prefix, branch_name)) < 0) + /* OOM */; + else if ((error = git_reference_lookup(&branch, repo, ref_name.ptr)) < 0) + giterr_set( + GITERR_REFERENCE, "Cannot locate %s branch '%s'", + is_remote ? "remote-tracking" : "local", branch_name); - if ((error = git_reference_lookup(&branch, repo, ref_name.ptr)) < 0) { - giterr_set(GITERR_REFERENCE, - "Cannot locate %s branch '%s'.", is_remote ? "remote-tracking" : "local", branch_name); - goto cleanup; - } + *branch_reference_out = branch; /* will be NULL on error */ - *branch_reference_out = branch; - -cleanup: git_buf_free(&ref_name); return error; } @@ -63,21 +58,19 @@ int git_branch_create( { git_reference *branch = NULL; git_buf canonical_branch_name = GIT_BUF_INIT; - int error = -1; + int error = 0; assert(branch_name && commit && ref_out); assert(git_object_owner((const git_object *)commit) == repository); - if (git_buf_joinpath(&canonical_branch_name, GIT_REFS_HEADS_DIR, branch_name) < 0) - goto cleanup; + if (!(error = git_buf_joinpath( + &canonical_branch_name, GIT_REFS_HEADS_DIR, branch_name))) + error = git_reference_create( + &branch, repository, git_buf_cstr(&canonical_branch_name), + git_commit_id(commit), force, NULL, NULL); - error = git_reference_create(&branch, repository, - git_buf_cstr(&canonical_branch_name), git_commit_id(commit), force, NULL, NULL); + *ref_out = branch; - if (!error) - *ref_out = branch; - -cleanup: git_buf_free(&canonical_branch_name); return error; } diff --git a/tests/status/renames.c b/tests/status/renames.c index a81910e36..df5e23087 100644 --- a/tests/status/renames.c +++ b/tests/status/renames.c @@ -560,7 +560,6 @@ void test_status_renames__zero_byte_file_does_not_fail(void) { git_status_list *statuslist; git_status_options opts = GIT_STATUS_OPTIONS_INIT; - status_entry_counts counts = {0}; struct status_entry expected[] = { { GIT_STATUS_WT_DELETED, "ikeepsix.txt", "ikeepsix.txt" }, From e9d5e5f3d41846b5f3be1bfb6a5cb1e501f428c8 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 28 Jan 2014 16:25:42 -0800 Subject: [PATCH 4/7] Some fixes for Windows x64 warnings --- include/git2/blame.h | 2 +- src/blame.c | 2 +- src/indexer.c | 2 +- src/pack-objects.c | 2 +- src/userdiff.h | 6 +++--- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/git2/blame.h b/include/git2/blame.h index 73bcc5bc6..b98c6f0d7 100644 --- a/include/git2/blame.h +++ b/include/git2/blame.h @@ -183,7 +183,7 @@ GIT_EXTERN(int) git_blame_buffer( git_blame **out, git_blame *reference, const char *buffer, - uint32_t buffer_len); + size_t buffer_len); /** * Free memory allocated by git_blame_file or git_blame_buffer. diff --git a/src/blame.c b/src/blame.c index 0f2d906d2..71bc460c6 100644 --- a/src/blame.c +++ b/src/blame.c @@ -450,7 +450,7 @@ int git_blame_buffer( git_blame **out, git_blame *reference, const char *buffer, - uint32_t buffer_len) + size_t buffer_len) { git_blame *blame; git_diff_options diffopts = GIT_DIFF_OPTIONS_INIT; diff --git a/src/indexer.c b/src/indexer.c index 9b60ef413..346870faa 100644 --- a/src/indexer.c +++ b/src/indexer.c @@ -659,7 +659,7 @@ static int inject_object(git_indexer *idx, git_oid *id) hdr_len = git_packfile__object_header(hdr, len, git_odb_object_type(obj)); git_filebuf_write(&idx->pack_file, hdr, hdr_len); idx->pack->mwf.size += hdr_len; - entry->crc = crc32(entry->crc, hdr, hdr_len); + entry->crc = crc32(entry->crc, hdr, (uInt)hdr_len); if ((error = git_zstream_deflatebuf(&buf, data, len)) < 0) goto cleanup; diff --git a/src/pack-objects.c b/src/pack-objects.c index 1774b07dc..8475f64f2 100644 --- a/src/pack-objects.c +++ b/src/pack-objects.c @@ -348,7 +348,7 @@ static int write_object( } if (written < 0) { - error = written; + error = (int)written; goto done; } diff --git a/src/userdiff.h b/src/userdiff.h index 9318b5476..7eb095246 100644 --- a/src/userdiff.h +++ b/src/userdiff.h @@ -193,9 +193,9 @@ PATTERNS("php", "|[-+*/<>%&^|=!]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\||::|->"), PATTERNS("javascript", - "^[ \t]*(function[ \t][a-zA-Z_][^\{]*)\n" - "^[ \t]*(var[ \t]+[a-zA-Z_][a-zA-Z0-9_]*[ \t]*=[ \t]*function[ \t\(][^\{]*)\n" - "^[ \t]*([a-zA-Z_][a-zA-Z0-9_]*[ \t]*:[ \t]*function[ \t\(][^\{]*)", + "^[ \t]*(function[ \t][a-zA-Z_][^\\{]*)\n" + "^[ \t]*(var[ \t]+[a-zA-Z_][a-zA-Z0-9_]*[ \t]*=[ \t]*function[ \t\\(][^\\{]*)\n" + "^[ \t]*([a-zA-Z_][a-zA-Z0-9_]*[ \t]*:[ \t]*function[ \t\\(][^\\{]*)", /* -- */ "[a-zA-Z_][a-zA-Z0-9_]*" "|[-+0-9.e]+[fFlL]?|0[xX]?[0-9a-fA-F]+[lL]?" From d9b04d78a396f771eada206b726ad6e8259a19b8 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 29 Jan 2014 15:02:35 -0800 Subject: [PATCH 5/7] Reorganize zstream API and fix wrap problems There were some confusing issues mixing up the number of bytes written to the zstream output buffer with the number of bytes consumed from the zstream input. This reorganizes the zstream API and makes it easier to deflate an arbitrarily large input while still using a fixed size output. --- src/pack-objects.c | 17 ++--- src/zstream.c | 162 +++++++++++++++++++++++++++++-------------- src/zstream.h | 21 ++++-- tests/core/zstream.c | 98 ++++++++++++++++++++++++++ 4 files changed, 231 insertions(+), 67 deletions(-) create mode 100644 tests/core/zstream.c diff --git a/src/pack-objects.c b/src/pack-objects.c index 8475f64f2..8b65ac26a 100644 --- a/src/pack-objects.c +++ b/src/pack-objects.c @@ -291,7 +291,6 @@ static int write_object( void *delta_data = NULL; void *data; size_t hdr_len, zbuf_len = COMPRESS_BUFLEN, data_len; - ssize_t written; int error; if (po->delta) { @@ -337,19 +336,15 @@ static int write_object( GITERR_CHECK_ALLOC(zbuf); git_zstream_reset(&pb->zstream); + git_zstream_set_input(&pb->zstream, data, data_len); - while ((written = git_zstream_deflate(zbuf, zbuf_len, &pb->zstream, data, data_len)) > 0) { - if ((error = write_cb(zbuf, written, cb_data)) < 0 || - (error = git_hash_update(&pb->ctx, zbuf, written)) < 0) + while (!git_zstream_done(&pb->zstream)) { + if ((error = git_zstream_get_output(zbuf, &zbuf_len, &pb->zstream)) < 0 || + (error = write_cb(zbuf, zbuf_len, cb_data)) < 0 || + (error = git_hash_update(&pb->ctx, zbuf, zbuf_len)) < 0) goto done; - data = (char *)data + written; - data_len -= written; - } - - if (written < 0) { - error = (int)written; - goto done; + zbuf_len = COMPRESS_BUFLEN; /* reuse buffer */ } if (po->delta) diff --git a/src/zstream.c b/src/zstream.c index 0bca72ff3..c83602297 100644 --- a/src/zstream.c +++ b/src/zstream.c @@ -10,14 +10,18 @@ #include "zstream.h" #include "buffer.h" -#define BUFFER_SIZE (1024 * 1024) +#define ZSTREAM_BUFFER_SIZE (1024 * 1024) +#define ZSTREAM_BUFFER_MIN_EXTRA 8 -static int zstream_seterr(int zerr, git_zstream *zstream) +static int zstream_seterr(git_zstream *zs) { - if (zerr == Z_MEM_ERROR) + if (zs->zerr == Z_OK || zs->zerr == Z_STREAM_END) + return 0; + + if (zs->zerr == Z_MEM_ERROR) giterr_set_oom(); - else if (zstream->msg) - giterr_set(GITERR_ZLIB, zstream->msg); + else if (zs->z.msg) + giterr_set(GITERR_ZLIB, zs->z.msg); else giterr_set(GITERR_ZLIB, "Unknown compression error"); @@ -26,69 +30,123 @@ static int zstream_seterr(int zerr, git_zstream *zstream) int git_zstream_init(git_zstream *zstream) { - int zerr; - - if ((zerr = deflateInit(zstream, Z_DEFAULT_COMPRESSION)) != Z_OK) - return zstream_seterr(zerr, zstream); - - return 0; -} - -ssize_t git_zstream_deflate(void *out, size_t out_len, git_zstream *zstream, const void *in, size_t in_len) -{ - int zerr; - - if ((ssize_t)out_len < 0) - out_len = INT_MAX; - - zstream->next_in = (Bytef *)in; - zstream->avail_in = in_len; - zstream->next_out = out; - zstream->avail_out = out_len; - - if ((zerr = deflate(zstream, Z_FINISH)) == Z_STREAM_ERROR) - return zstream_seterr(zerr, zstream); - - return (out_len - zstream->avail_out); -} - -void git_zstream_reset(git_zstream *zstream) -{ - deflateReset(zstream); + zstream->zerr = deflateInit(&zstream->z, Z_DEFAULT_COMPRESSION); + return zstream_seterr(zstream); } void git_zstream_free(git_zstream *zstream) { - deflateEnd(zstream); + deflateEnd(&zstream->z); +} + +void git_zstream_reset(git_zstream *zstream) +{ + deflateReset(&zstream->z); + zstream->in = NULL; + zstream->in_len = 0; + zstream->zerr = Z_STREAM_END; +} + +int git_zstream_set_input(git_zstream *zstream, const void *in, size_t in_len) +{ + zstream->in = in; + zstream->in_len = in_len; + zstream->zerr = Z_OK; + return 0; +} + +bool git_zstream_done(git_zstream *zstream) +{ + return (!zstream->in_len && zstream->zerr == Z_STREAM_END); +} + +size_t git_zstream_suggest_output_len(git_zstream *zstream) +{ + if (zstream->in_len > ZSTREAM_BUFFER_SIZE) + return ZSTREAM_BUFFER_SIZE; + else if (zstream->in_len > ZSTREAM_BUFFER_MIN_EXTRA) + return zstream->in_len; + else + return ZSTREAM_BUFFER_MIN_EXTRA; +} + +int git_zstream_get_output(void *out, size_t *out_len, git_zstream *zstream) +{ + int zflush = Z_FINISH; + size_t out_remain = *out_len; + + while (out_remain > 0 && zstream->zerr != Z_STREAM_END) { + size_t out_queued, in_queued, out_used, in_used; + + /* set up in data */ + zstream->z.next_in = (Bytef *)zstream->in; + zstream->z.avail_in = (uInt)zstream->in_len; + if ((size_t)zstream->z.avail_in != zstream->in_len) { + zstream->z.avail_in = INT_MAX; + zflush = Z_NO_FLUSH; + } else { + zflush = Z_FINISH; + } + in_queued = (size_t)zstream->z.avail_in; + + /* set up out data */ + zstream->z.next_out = out; + zstream->z.avail_out = (uInt)out_remain; + if ((size_t)zstream->z.avail_out != out_remain) + zstream->z.avail_out = INT_MAX; + out_queued = (size_t)zstream->z.avail_out; + + /* compress next chunk */ + zstream->zerr = deflate(&zstream->z, zflush); + if (zstream->zerr == Z_STREAM_ERROR) + return zstream_seterr(zstream); + + out_used = (out_queued - zstream->z.avail_out); + out_remain -= out_used; + out = ((char *)out) + out_used; + + in_used = (in_queued - zstream->z.avail_in); + zstream->in_len -= in_used; + zstream->in += in_used; + } + + /* either we finished the input or we did not flush the data */ + assert(zstream->in_len > 0 || zflush == Z_FINISH); + + /* set out_size to number of bytes actually written to output */ + *out_len = *out_len - out_remain; + + return 0; } int git_zstream_deflatebuf(git_buf *out, const void *in, size_t in_len) { - git_zstream zstream = GIT_ZSTREAM_INIT; - size_t out_len; - ssize_t written; + git_zstream zs = GIT_ZSTREAM_INIT; int error = 0; - if ((error = git_zstream_init(&zstream)) < 0) + if ((error = git_zstream_init(&zs)) < 0) return error; - do { - if (out->asize - out->size < BUFFER_SIZE) - git_buf_grow(out, out->asize + BUFFER_SIZE); + if ((error = git_zstream_set_input(&zs, in, in_len)) < 0) + goto done; - out_len = out->asize - out->size; + while (!git_zstream_done(&zs)) { + size_t step = git_zstream_suggest_output_len(&zs), written; - if ((written = git_zstream_deflate(out->ptr + out->size, out_len, &zstream, in, in_len)) <= 0) - break; + if ((error = git_buf_grow(out, out->asize + step)) < 0) + goto done; + + written = out->asize - out->size; + + if ((error = git_zstream_get_output( + out->ptr + out->size, &written, &zs)) < 0) + goto done; - in = (char *)in + written; - in_len -= written; out->size += written; - } while (written > 0); + out->ptr[out->size] = '\0'; + } - if (written < 0) - error = written; - - git_zstream_free(&zstream); +done: + git_zstream_free(&zs); return error; } diff --git a/src/zstream.h b/src/zstream.h index 9672903c0..9b5bf6ace 100644 --- a/src/zstream.h +++ b/src/zstream.h @@ -12,15 +12,28 @@ #include "common.h" #include "buffer.h" -#define git_zstream z_stream +typedef struct { + z_stream z; + const char *in; + size_t in_len; + int zerr; +} git_zstream; -#define GIT_ZSTREAM_INIT {0} +#define GIT_ZSTREAM_INIT {{0}} int git_zstream_init(git_zstream *zstream); -ssize_t git_zstream_deflate(void *out, size_t out_len, git_zstream *zstream, const void *in, size_t in_len); -void git_zstream_reset(git_zstream *zstream); void git_zstream_free(git_zstream *zstream); +int git_zstream_set_input(git_zstream *zstream, const void *in, size_t in_len); + +size_t git_zstream_suggest_output_len(git_zstream *zstream); + +int git_zstream_get_output(void *out, size_t *out_len, git_zstream *zstream); + +bool git_zstream_done(git_zstream *zstream); + +void git_zstream_reset(git_zstream *zstream); + int git_zstream_deflatebuf(git_buf *out, const void *in, size_t in_len); #endif /* INCLUDE_zstream_h__ */ diff --git a/tests/core/zstream.c b/tests/core/zstream.c new file mode 100644 index 000000000..63ff8c93a --- /dev/null +++ b/tests/core/zstream.c @@ -0,0 +1,98 @@ +#include "clar_libgit2.h" +#include "buffer.h" +#include "zstream.h" + +static const char *data = "This is a test test test of This is a test"; + +#define INFLATE_EXTRA 2 + +static void assert_zlib_equal_( + const void *expected, size_t e_len, + const void *compressed, size_t c_len, + const char *msg, const char *file, int line) +{ + z_stream stream; + char *expanded = git__calloc(1, e_len + INFLATE_EXTRA); + cl_assert(expanded); + + memset(&stream, 0, sizeof(stream)); + stream.next_out = (Bytef *)expanded; + stream.avail_out = (uInt)(e_len + INFLATE_EXTRA); + stream.next_in = (Bytef *)compressed; + stream.avail_in = (uInt)c_len; + + cl_assert(inflateInit(&stream) == Z_OK); + cl_assert(inflate(&stream, Z_FINISH)); + inflateEnd(&stream); + + clar__assert_equal( + file, line, msg, 1, + "%d", (int)stream.total_out, (int)e_len); + clar__assert_equal( + file, line, "Buffer len was not exact match", 1, + "%d", (int)stream.avail_out, (int)INFLATE_EXTRA); + + clar__assert( + memcmp(expanded, expected, e_len) == 0, + file, line, "uncompressed data did not match", NULL, 1); + + git__free(expanded); +} + +#define assert_zlib_equal(E,EL,C,CL) \ + assert_zlib_equal_(E, EL, C, CL, #EL " != " #CL, __FILE__, (int)__LINE__) + +void test_core_zstream__basic(void) +{ + git_zstream z = GIT_ZSTREAM_INIT; + char out[128]; + size_t outlen = sizeof(out); + + cl_git_pass(git_zstream_init(&z)); + cl_git_pass(git_zstream_set_input(&z, data, strlen(data) + 1)); + cl_git_pass(git_zstream_get_output(out, &outlen, &z)); + cl_assert(git_zstream_done(&z)); + cl_assert(outlen > 0); + git_zstream_free(&z); + + assert_zlib_equal(data, strlen(data) + 1, out, outlen); +} + +void test_core_zstream__buffer(void) +{ + git_buf out = GIT_BUF_INIT; + cl_git_pass(git_zstream_deflatebuf(&out, data, strlen(data) + 1)); + assert_zlib_equal(data, strlen(data) + 1, out.ptr, out.size); + git_buf_free(&out); +} + +#define BIG_STRING_PART "Big Data IS Big - Long Data IS Long - We need a buffer larger than 1024 x 1024 to make sure we trigger chunked compression - Big Big Data IS Bigger than Big - Long Long Data IS Longer than Long" + +void test_core_zstream__big_data(void) +{ + git_buf in = GIT_BUF_INIT; + git_buf out = GIT_BUF_INIT; + size_t scan; + + /* make a big string that's easy to compress */ + while (in.size < 1024 * 1024) + cl_git_pass(git_buf_put(&in, BIG_STRING_PART, strlen(BIG_STRING_PART))); + + cl_git_pass(git_zstream_deflatebuf(&out, in.ptr, in.size)); + assert_zlib_equal(in.ptr, in.size, out.ptr, out.size); + + git_buf_free(&out); + + /* make a big string that's hard to compress */ + + srand(0xabad1dea); + for (scan = 0; scan < in.size; ++scan) + in.ptr[scan] = (char)rand(); + + cl_git_pass(git_zstream_deflatebuf(&out, in.ptr, in.size)); + assert_zlib_equal(in.ptr, in.size, out.ptr, out.size); + + git_buf_free(&out); + + git_buf_free(&in); +} From 8606f33beadf5df48b36a64359c99d50aeb0f496 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 30 Jan 2014 09:59:15 -0800 Subject: [PATCH 6/7] Expand zstream tests and fix off-by-one error --- src/zstream.c | 3 +- tests/core/zstream.c | 79 ++++++++++++++++++++++++++++++++++---------- 2 files changed, 64 insertions(+), 18 deletions(-) diff --git a/src/zstream.c b/src/zstream.c index c83602297..82ae5e6f0 100644 --- a/src/zstream.c +++ b/src/zstream.c @@ -98,6 +98,7 @@ int git_zstream_get_output(void *out, size_t *out_len, git_zstream *zstream) /* compress next chunk */ zstream->zerr = deflate(&zstream->z, zflush); + if (zstream->zerr == Z_STREAM_ERROR) return zstream_seterr(zstream); @@ -133,7 +134,7 @@ int git_zstream_deflatebuf(git_buf *out, const void *in, size_t in_len) while (!git_zstream_done(&zs)) { size_t step = git_zstream_suggest_output_len(&zs), written; - if ((error = git_buf_grow(out, out->asize + step)) < 0) + if ((error = git_buf_grow(out, out->asize + step + 1)) < 0) goto done; written = out->asize - out->size; diff --git a/tests/core/zstream.c b/tests/core/zstream.c index 63ff8c93a..7ba9424ba 100644 --- a/tests/core/zstream.c +++ b/tests/core/zstream.c @@ -68,31 +68,76 @@ void test_core_zstream__buffer(void) #define BIG_STRING_PART "Big Data IS Big - Long Data IS Long - We need a buffer larger than 1024 x 1024 to make sure we trigger chunked compression - Big Big Data IS Bigger than Big - Long Long Data IS Longer than Long" +static void compress_input_various_ways(git_buf *input) +{ + git_buf out1 = GIT_BUF_INIT, out2 = GIT_BUF_INIT; + size_t i, fixed_size = max(input->size / 2, 256); + char *fixed = git__malloc(fixed_size); + cl_assert(fixed); + + /* compress with deflatebuf */ + + cl_git_pass(git_zstream_deflatebuf(&out1, input->ptr, input->size)); + assert_zlib_equal(input->ptr, input->size, out1.ptr, out1.size); + + /* compress with various fixed size buffer (accumulating the output) */ + + for (i = 0; i < 3; ++i) { + git_zstream zs = GIT_ZSTREAM_INIT; + size_t use_fixed_size; + + switch (i) { + case 0: use_fixed_size = 256; break; + case 1: use_fixed_size = fixed_size / 2; break; + case 2: use_fixed_size = fixed_size; break; + } + cl_assert(use_fixed_size <= fixed_size); + + cl_git_pass(git_zstream_init(&zs)); + cl_git_pass(git_zstream_set_input(&zs, input->ptr, input->size)); + + while (!git_zstream_done(&zs)) { + size_t written = use_fixed_size; + cl_git_pass(git_zstream_get_output(fixed, &written, &zs)); + cl_git_pass(git_buf_put(&out2, fixed, written)); + } + + git_zstream_free(&zs); + assert_zlib_equal(input->ptr, input->size, out2.ptr, out2.size); + + /* did both approaches give the same data? */ + cl_assert_equal_sz(out1.size, out2.size); + cl_assert(!memcmp(out1.ptr, out2.ptr, out1.size)); + + git_buf_free(&out2); + } + + git_buf_free(&out1); + git__free(fixed); +} + void test_core_zstream__big_data(void) { git_buf in = GIT_BUF_INIT; - git_buf out = GIT_BUF_INIT; - size_t scan; + size_t scan, target; - /* make a big string that's easy to compress */ - while (in.size < 1024 * 1024) - cl_git_pass(git_buf_put(&in, BIG_STRING_PART, strlen(BIG_STRING_PART))); + for (target = 1024; target <= 1024 * 1024 * 4; target *= 8) { - cl_git_pass(git_zstream_deflatebuf(&out, in.ptr, in.size)); - assert_zlib_equal(in.ptr, in.size, out.ptr, out.size); + /* make a big string that's easy to compress */ + git_buf_clear(&in); + while (in.size < target) + cl_git_pass( + git_buf_put(&in, BIG_STRING_PART, strlen(BIG_STRING_PART))); - git_buf_free(&out); + compress_input_various_ways(&in); - /* make a big string that's hard to compress */ + /* make a big string that's hard to compress */ + srand(0xabad1dea); + for (scan = 0; scan < in.size; ++scan) + in.ptr[scan] = (char)rand(); - srand(0xabad1dea); - for (scan = 0; scan < in.size; ++scan) - in.ptr[scan] = (char)rand(); - - cl_git_pass(git_zstream_deflatebuf(&out, in.ptr, in.size)); - assert_zlib_equal(in.ptr, in.size, out.ptr, out.size); - - git_buf_free(&out); + compress_input_various_ways(&in); + } git_buf_free(&in); } From 19459b1e29e150c02e733bdefd5e1eb09fdd4bf7 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 30 Jan 2014 10:23:35 -0800 Subject: [PATCH 7/7] Defer zstream NUL termination to end And don't terminate if there isn't space for it (since it's binary data, it's not worth a reallocation). --- src/zstream.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/zstream.c b/src/zstream.c index 82ae5e6f0..85fa2e0e6 100644 --- a/src/zstream.c +++ b/src/zstream.c @@ -134,7 +134,7 @@ int git_zstream_deflatebuf(git_buf *out, const void *in, size_t in_len) while (!git_zstream_done(&zs)) { size_t step = git_zstream_suggest_output_len(&zs), written; - if ((error = git_buf_grow(out, out->asize + step + 1)) < 0) + if ((error = git_buf_grow(out, out->asize + step)) < 0) goto done; written = out->asize - out->size; @@ -144,9 +144,12 @@ int git_zstream_deflatebuf(git_buf *out, const void *in, size_t in_len) goto done; out->size += written; - out->ptr[out->size] = '\0'; } + /* NULL terminate for consistency if possible */ + if (out->size < out->asize) + out->ptr[out->size] = '\0'; + done: git_zstream_free(&zs); return error;