Skip to content

Commit

Permalink
apply: use new promise structures in git-apply logic as a proving ground
Browse files Browse the repository at this point in the history
  • Loading branch information
philip-peterson committed Feb 5, 2024
1 parent 280ef74 commit f9f0558
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 54 deletions.
127 changes: 81 additions & 46 deletions apply.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "object-file.h"
#include "parse-options.h"
#include "path.h"
#include "promise.h"
#include "quote.h"
#include "read-cache.h"
#include "rerere.h"
Expand Down Expand Up @@ -1317,13 +1318,14 @@ static int check_header_line(int linenr, struct patch *patch)
return 0;
}

int parse_git_diff_header(struct strbuf *root,
void parse_git_diff_header(struct strbuf *root,
int *linenr,
int p_value,
const char *line,
int len,
unsigned int size,
struct patch *patch)
struct patch *patch,
struct promise_t* promise)
{
unsigned long offset;
struct gitdiff_data parse_hdr_state;
Expand Down Expand Up @@ -1387,10 +1389,12 @@ int parse_git_diff_header(struct strbuf *root,
if (len < oplen || memcmp(p->str, line, oplen))
continue;
res = p->fn(&parse_hdr_state, line + oplen, patch);
if (res < 0)
return -1;
if (check_header_line(*linenr, patch))
return -1;
if (res < 0) {
PROMISE_THROW(promise, APPLY_ERR_GENERIC, "operation for \"%s\" failed with code: %d", p->str, res);
}
if (check_header_line(*linenr, patch)) {
PROMISE_THROW(promise, APPLY_ERR_GENERIC, "invalid header lines");
}
if (res > 0)
goto done;
break;
Expand All @@ -1400,25 +1404,25 @@ int parse_git_diff_header(struct strbuf *root,
done:
if (!patch->old_name && !patch->new_name) {
if (!patch->def_name) {
error(Q_("git diff header lacks filename information when removing "
"%d leading pathname component (line %d)",
"git diff header lacks filename information when removing "
"%d leading pathname components (line %d)",
parse_hdr_state.p_value),
parse_hdr_state.p_value, *linenr);
return -128;
PROMISE_THROW(promise, -128,
Q_("git diff header lacks filename information when removing "
"%d leading pathname component (line %d)",
"git diff header lacks filename information when removing "
"%d leading pathname components (line %d)",
parse_hdr_state.p_value),
parse_hdr_state.p_value, *linenr
);
}
patch->old_name = xstrdup(patch->def_name);
patch->new_name = xstrdup(patch->def_name);
}
if ((!patch->new_name && !patch->is_delete) ||
(!patch->old_name && !patch->is_new)) {
error(_("git diff header lacks filename information "
PROMISE_THROW(promise, -128, _("git diff header lacks filename information "
"(line %d)"), *linenr);
return -128;
}
patch->is_toplevel_relative = 1;
return offset;
PROMISE_SUCCEED(promise, offset);
}

static int parse_num(const char *line, unsigned long *p)
Expand Down Expand Up @@ -1540,16 +1544,17 @@ static int parse_fragment_header(const char *line, int len, struct fragment *fra
/*
* Find file diff header
*
* Returns:
* Resolves promise with:
* -1 if no header was found
* -128 in case of error
* the size of the header in bytes (called "offset") otherwise
*/
static int find_header(struct apply_state *state,
static void find_header(struct apply_state *state,
const char *line,
unsigned long size,
int *hdrsize,
struct patch *patch)
struct patch *patch,
struct promise_t* promise)
{
unsigned long offset, len;

Expand Down Expand Up @@ -1578,9 +1583,8 @@ static int find_header(struct apply_state *state,
struct fragment dummy;
if (parse_fragment_header(line, len, &dummy) < 0)
continue;
error(_("patch fragment without header at line %d: %.*s"),
PROMISE_THROW(promise, -128, _("patch fragment without header at line %d: %.*s"),
state->linenr, (int)len-1, line);
return -128;
}

if (size < len + 6)
Expand All @@ -1591,15 +1595,23 @@ static int find_header(struct apply_state *state,
* or mode change, so we handle that specially
*/
if (!memcmp("diff --git ", line, 11)) {
int git_hdr_len = parse_git_diff_header(&state->root, &state->linenr,
struct promise_t *parse_git_diff_header_promise = promise_init();
parse_git_diff_header(&state->root, &state->linenr,
state->p_value, line, len,
size, patch);
if (git_hdr_len < 0)
return -128;
size, patch, parse_git_diff_header_promise);
promise_assert_finished(parse_git_diff_header_promise);

if (parse_git_diff_header_promise->state == PROMISE_FAILURE) {
PROMISE_BUBBLE_UP(promise, parse_git_diff_header_promise,
_("could not find file diff header"));
}

Check failure on line 1608 in apply.c

View workflow job for this annotation

GitHub Actions / linux-musl (alpine)

apply.c:1608:25: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]

Check failure on line 1608 in apply.c

View workflow job for this annotation

GitHub Actions / pedantic (fedora)

apply.c:1608:25: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]

Check failure on line 1608 in apply.c

View workflow job for this annotation

GitHub Actions / linux-asan-ubsan (ubuntu-latest)

apply.c:1608:8: mixing declarations and code is incompatible with standards before C99 [-Werror,-Wdeclaration-after-statement]

Check failure on line 1608 in apply.c

View workflow job for this annotation

GitHub Actions / linux-gcc (ubuntu-20.04)

apply.c:1608:4: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]

Check failure on line 1608 in apply.c

View workflow job for this annotation

GitHub Actions / linux-gcc-default (ubuntu-latest)

apply.c:1608:25: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]

Check failure on line 1608 in apply.c

View workflow job for this annotation

GitHub Actions / linux-leaks (ubuntu-latest)

apply.c:1608:25: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]

Check failure on line 1608 in apply.c

View workflow job for this annotation

GitHub Actions / linux-sha256 (ubuntu-latest)

apply.c:1608:8: mixing declarations and code is incompatible with standards before C99 [-Werror,-Wdeclaration-after-statement]

Check failure on line 1608 in apply.c

View workflow job for this annotation

GitHub Actions / linux-TEST-vars (ubuntu-20.04)

apply.c:1608:4: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
int git_hdr_len = promise->result.success_result;
if (git_hdr_len <= len)
continue;
*hdrsize = git_hdr_len;
return offset;
PROMISE_SUCCEED(promise, offset);
return;
}

/* --- followed by +++ ? */
Expand All @@ -1616,13 +1628,14 @@ static int find_header(struct apply_state *state,
continue;

/* Ok, we'll consider it a patch */
if (parse_traditional_patch(state, line, line+len, patch))
return -128;
if (parse_traditional_patch(state, line, line+len, patch)) {
PROMISE_THROW(promise, -128, "could not parse traditional patch");
}
*hdrsize = len + nextlen;
state->linenr += 2;
return offset;
PROMISE_SUCCEED(promise, offset);
}
return -1;
PROMISE_THROW(promise, APPLY_ERR_GENERIC, "no lines to read");
}

static void record_ws_error(struct apply_state *state,
Expand Down Expand Up @@ -2130,19 +2143,28 @@ static int use_patch(struct apply_state *state, struct patch *p)
* reading after seeing a single patch (i.e. changes to a single file).
* Create fragments (i.e. patch hunks) and hang them to the given patch.
*
* Returns:
* Resolves promise with:
* -1 if no header was found or parse_binary() failed,
* -128 on another error,
* the number of bytes consumed otherwise,
* so that the caller can call us again for the next patch.
*/
static int parse_chunk(struct apply_state *state, char *buffer, unsigned long size, struct patch *patch)
static void parse_chunk(struct apply_state *state, char *buffer, unsigned long size, struct patch *patch, struct promise_t* promise)
{

Check failure on line 2153 in apply.c

View workflow job for this annotation

GitHub Actions / linux-gcc (ubuntu-20.04)

apply.c:2153:6: ‘hdrsize’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

Check failure on line 2153 in apply.c

View workflow job for this annotation

GitHub Actions / linux-TEST-vars (ubuntu-20.04)

apply.c:2153:6: ‘hdrsize’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
int hdrsize, patchsize;
int offset = find_header(state, buffer, size, &hdrsize, patch);
int hdrsize, patchsize, offset;
struct promise_t *find_header_promise = promise_init();
find_header(state, buffer, size, &hdrsize, patch, find_header_promise);
promise_assert_finished(find_header_promise);

if (offset < 0)
return offset;
if (find_header_promise->state == PROMISE_FAILURE) {
PROMISE_BUBBLE_UP(promise, find_header_promise, _("could not find header"));
}

offset = find_header_promise->result.success_result;

if (offset < 0) {
PROMISE_SUCCEED(promise, offset);
}

prefix_patch(state, patch);

Expand All @@ -2160,8 +2182,9 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si
size - offset - hdrsize,
patch);

if (patchsize < 0)
return -128;
if (patchsize < 0) {
PROMISE_THROW(promise, -128, _("could not parse patch"));
}

if (!patchsize) {
static const char git_binary[] = "GIT binary patch\n";
Expand All @@ -2174,8 +2197,9 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si
state->linenr++;
used = parse_binary(state, buffer + hd + llen,
size - hd - llen, patch);
if (used < 0)
return -1;
if (used < 0) {
PROMISE_THROW(promise, -1, _("could not parse binary patch"));
}
if (used)
patchsize = used + llen;
else
Expand Down Expand Up @@ -2206,12 +2230,11 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si
*/
if ((state->apply || state->check) &&
(!patch->is_binary && !metadata_changes(patch))) {
error(_("patch with only garbage at line %d"), state->linenr);
return -128;
PROMISE_THROW(promise, -128, _("patch with only garbage at line %d"), state->linenr);
}
}

return offset + hdrsize + patchsize;
PROMISE_SUCCEED(promise, offset + hdrsize + patchsize);
}

static void reverse_patches(struct patch *p)
Expand Down Expand Up @@ -4756,21 +4779,33 @@ static int apply_patch(struct apply_state *state,
return -128;
offset = 0;
while (offset < buf.len) {
struct patch *patch;
int nr;
struct patch *patch;
struct promise_t *parse_chunk_promise;

CALLOC_ARRAY(patch, 1);
patch->inaccurate_eof = !!(options & APPLY_OPT_INACCURATE_EOF);
patch->recount = !!(options & APPLY_OPT_RECOUNT);
nr = parse_chunk(state, buf.buf + offset, buf.len - offset, patch);
if (nr < 0) {

parse_chunk_promise = promise_init();
parse_chunk(state, buf.buf + offset, buf.len - offset, patch, parse_chunk_promise);

promise_assert_finished(parse_chunk_promise);

if (parse_chunk_promise->state == PROMISE_FAILURE) {
int nr;
error("\n\t%s", parse_chunk_promise->result.failure_result.message.buf);

nr = parse_chunk_promise->result.failure_result.status;
free_patch(patch);
if (nr == -128) {
res = -128;
goto end;
}
break;
}

nr = parse_chunk_promise->result.success_result;
if (state->apply_in_reverse)
reverse_patches(patch);
if (use_patch(state, patch)) {
Expand Down
9 changes: 7 additions & 2 deletions apply.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
#include "lockfile.h"
#include "string-list.h"
#include "strmap.h"
#include "promise.h"

/* Error codes (must be less than 0) */
#define APPLY_ERR_GENERIC -1

struct repository;

Expand Down Expand Up @@ -165,13 +169,14 @@ int check_apply_state(struct apply_state *state, int force_apply);
*
* Returns -1 on failure, the length of the parsed header otherwise.
*/
int parse_git_diff_header(struct strbuf *root,
void parse_git_diff_header(struct strbuf *root,
int *linenr,
int p_value,
const char *line,
int len,
unsigned int size,
struct patch *patch);
struct patch *patch,
struct promise_t *promise);

void release_patch(struct patch *patch);

Expand Down
13 changes: 7 additions & 6 deletions range-diff.c
Original file line number Diff line number Diff line change
Expand Up @@ -121,25 +121,26 @@ static int read_patches(const char *range, struct string_list *list,
if (starts_with(line, "diff --git")) {
struct patch patch = { 0 };
struct strbuf root = STRBUF_INIT;
struct promise_t *parse_git_diff_header_promise = promise_init();
int linenr = 0;
int orig_len;

in_header = 0;
strbuf_addch(&buf, '\n');
if (!util->diff_offset)
util->diff_offset = buf.len;
if (eol)
*eol = '\n';
orig_len = len;
len = parse_git_diff_header(&root, &linenr, 0, line,
len, size, &patch);
if (len < 0) {
parse_git_diff_header(&root, &linenr, 0, line,
len, size, &patch, parse_git_diff_header_promise);
promise_assert_finished(parse_git_diff_header_promise);
if (parse_git_diff_header_promise->state == PROMISE_FAILURE) {
error(_("could not parse git header '%.*s'"),

Check failure on line 137 in range-diff.c

View workflow job for this annotation

GitHub Actions / fuzz smoke test

range-diff.c:137:44: field precision should have type 'int', but argument has type 'ssize_t' (aka 'long') [-Werror,-Wformat]

Check failure on line 137 in range-diff.c

View workflow job for this annotation

GitHub Actions / win build

range-diff.c:137:41: field precision specifier '.*' expects argument of type 'int', but argument 2 has type 'ssize_t' {aka 'long long int'} [-Werror=format=]
orig_len, line);
len, line);
FREE_AND_NULL(util);
string_list_clear(list, 1);
goto cleanup;
}
len = parse_git_diff_header_promise->result.success_result;
strbuf_addstr(&buf, " ## ");
if (patch.is_new > 0)
strbuf_addf(&buf, "%s (new)", patch.new_name);
Expand Down

0 comments on commit f9f0558

Please sign in to comment.