From 19853bdd97e006b6e4519bc352c3e8fd7586e9c3 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 10 Dec 2013 13:01:34 -0800 Subject: [PATCH] Update git_blob_create_fromchunks callback behavr The callback to supply data chunks could return a negative value to stop creation of the blob, but we were neither using GIT_EUSER nor propagating the return value. This makes things use the new behavior of returning the negative value back to the user. --- include/git2/blob.h | 43 +++++++++++++++------------------- src/blob.c | 34 +++++++++++++++++---------- tests/object/blob/fromchunks.c | 39 +++++++++++++++++++++++++++++- 3 files changed, 78 insertions(+), 38 deletions(-) diff --git a/include/git2/blob.h b/include/git2/blob.h index dcab4fbe0..19ad4d949 100644 --- a/include/git2/blob.h +++ b/include/git2/blob.h @@ -159,37 +159,32 @@ typedef int (*git_blob_chunk_cb)(char *content, size_t max_length, void *payload * Write a loose blob to the Object Database from a * provider of chunks of data. * - * Provided the `hintpath` parameter is filled, its value - * will help to determine what git filters should be applied - * to the object before it can be placed to the object database. + * If the `hintpath` parameter is filled, it will be used to determine + * what git filters should be applied to the object before it is written + * to the object database. * + * The implementation of the callback MUST respect the following rules: * - * The implementation of the callback has to respect the - * following rules: + * - `content` must be filled by the callback. The maximum number of + * bytes that the buffer can accept per call is defined by the + * `max_length` parameter. Allocation and freeing of the buffer will + * be taken care of by libgit2. * - * - `content` will have to be filled by the consumer. The maximum number - * of bytes that the buffer can accept per call is defined by the - * `max_length` parameter. Allocation and freeing of the buffer will be taken - * care of by the function. + * - The `callback` must return the number of bytes that have been + * written to the `content` buffer. * - * - The callback is expected to return the number of bytes - * that `content` have been filled with. - * - * - When there is no more data to stream, the callback should - * return 0. This will prevent it from being invoked anymore. - * - * - When an error occurs, the callback should return -1. + * - When there is no more data to stream, `callback` should return + * 0. This will prevent it from being invoked anymore. * + * - If an error occurs, the callback should return a negative value. + * This value will be returned to the caller. * * @param id Return the id of the written blob - * - * @param repo repository where the blob will be written. - * This repository can be bare or not. - * - * @param hintpath if not NULL, will help selecting the filters - * to apply onto the content of the blob to be created. - * - * @return 0 or an error code + * @param repo Repository where the blob will be written. + * This repository can be bare or not. + * @param hintpath If not NULL, will be used to select data filters + * to apply onto the content of the blob to be created. + * @return 0 or error code (from either libgit2 or callback function) */ GIT_EXTERN(int) git_blob_create_fromchunks( git_oid *id, diff --git a/src/blob.c b/src/blob.c index 2c6d52800..ab344ae98 100644 --- a/src/blob.c +++ b/src/blob.c @@ -272,37 +272,44 @@ int git_blob_create_fromchunks( int (*source_cb)(char *content, size_t max_length, void *payload), void *payload) { - int error = -1, read_bytes; + int error; char *content = NULL; git_filebuf file = GIT_FILEBUF_INIT; git_buf path = GIT_BUF_INIT; - if (git_buf_joinpath( - &path, git_repository_path(repo), GIT_OBJECTS_DIR "streamed") < 0) + assert(oid && repo && source_cb); + + if ((error = git_buf_joinpath( + &path, git_repository_path(repo), GIT_OBJECTS_DIR "streamed")) < 0) goto cleanup; content = git__malloc(BUFFER_SIZE); GITERR_CHECK_ALLOC(content); - if (git_filebuf_open(&file, git_buf_cstr(&path), GIT_FILEBUF_TEMPORARY, 0666) < 0) + if ((error = git_filebuf_open( + &file, git_buf_cstr(&path), GIT_FILEBUF_TEMPORARY, 0666)) < 0) goto cleanup; while (1) { - read_bytes = source_cb(content, BUFFER_SIZE, payload); + int read_bytes = source_cb(content, BUFFER_SIZE, payload); - assert(read_bytes <= BUFFER_SIZE); - - if (read_bytes <= 0) + if (!read_bytes) break; - if (git_filebuf_write(&file, content, read_bytes) < 0) + if (read_bytes > BUFFER_SIZE) { + giterr_set(GITERR_OBJECT, "Invalid chunk size while creating blob"); + error = GIT_EBUFS; + } else if (read_bytes < 0) { + error = giterr_set_after_callback(read_bytes); + } else { + error = git_filebuf_write(&file, content, read_bytes); + } + + if (error < 0) goto cleanup; } - if (read_bytes < 0) - goto cleanup; - - if (git_filebuf_flush(&file) < 0) + if ((error = git_filebuf_flush(&file)) < 0) goto cleanup; error = git_blob__create_from_paths( @@ -312,6 +319,7 @@ cleanup: git_buf_free(&path); git_filebuf_cleanup(&file); git__free(content); + return error; } diff --git a/tests/object/blob/fromchunks.c b/tests/object/blob/fromchunks.c index 03ed4efb4..b61cabfe1 100644 --- a/tests/object/blob/fromchunks.c +++ b/tests/object/blob/fromchunks.c @@ -59,7 +59,7 @@ void test_object_blob_fromchunks__doesnot_overwrite_an_already_existing_object(v git_buf content = GIT_BUF_INIT; git_oid expected_oid, oid; int howmany = 7; - + cl_git_pass(git_oid_fromstr(&expected_oid, "321cbdf08803c744082332332838df6bd160f8f9")); cl_git_pass(git_blob_create_fromchunks(&oid, repo, NULL, text_chunked_source_cb, &howmany)); @@ -117,3 +117,40 @@ void test_object_blob_fromchunks__creating_a_blob_from_chunks_honors_the_attribu assert_named_chunked_blob("e9671e138a780833cb689753570fd10a55be84fb", "dummy.txt"); assert_named_chunked_blob("e9671e138a780833cb689753570fd10a55be84fb", "dummy.dunno"); } + +static int failing_chunked_source_cb( + char *content, size_t max_length, void *payload) +{ + int *count = (int *)payload; + + GIT_UNUSED(max_length); + + (*count)--; + if (*count == 0) + return -1234; + + strcpy(content, textual_content); + return (int)strlen(textual_content); +} + +void test_object_blob_fromchunks__can_stop_with_error(void) +{ + git_oid expected_oid, oid; + git_object *blob; + int howmany = 7; + + cl_git_pass(git_oid_fromstr( + &expected_oid, "321cbdf08803c744082332332838df6bd160f8f9")); + + cl_git_fail_with( + git_object_lookup(&blob, repo, &expected_oid, GIT_OBJ_ANY), + GIT_ENOTFOUND); + + cl_git_fail_with(git_blob_create_fromchunks( + &oid, repo, NULL, failing_chunked_source_cb, &howmany), -1234); + + cl_git_fail_with( + git_object_lookup(&blob, repo, &expected_oid, GIT_OBJ_ANY), + GIT_ENOTFOUND); +} +