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.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185
diff --git a/include/git2/blob.h b/include/git2/blob.h
index dcab4fb..19ad4d9 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 2c6d528..ab344ae 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);
-
- assert(read_bytes <= BUFFER_SIZE);
+ int read_bytes = source_cb(content, BUFFER_SIZE, payload);
- 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 03ed4ef..b61cabf 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);
+}
+