This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1028

mpatch: reformat function prototypes with clang-format
ClosedPublic

Authored by durin42 on Oct 12 2017, 11:09 AM.

Details

Reviewers
pulkit
yuja
Group Reviewers
hg-reviewers
Commits
rHGbd177139e635: mpatch: reformat function prototypes with clang-format
Summary

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

durin42 created this revision.Oct 12 2017, 11:09 AM
pulkit accepted this revision.Oct 12 2017, 1:47 PM
yuja requested changes to this revision.Oct 13 2017, 9:19 AM
yuja added a subscriber: yuja.

Seemed fine, but test-check-code.t says no.

+  mercurial/mpatch.c:268:
+   >             ssize_t start, ssize_t end)
+   don't use spaces to indent
+  mercurial/mpatch.h:24:
+   >             ssize_t start, ssize_t end);
+   don't use spaces to indent
This revision now requires changes to proceed.Oct 13 2017, 9:19 AM
ryanmce added a subscriber: ryanmce.Oct 13 2017, 1:40 PM

I thought it was string that check-code doesn't fire on D1030 but does fire here.

It turns out that the check-code only finds spaces at the start of the line. So is you start with a tab and then spaces, that's fine. But if you start with spaces you trip up against test-check-code. In D1030, the re-indented line start with a tab, whereas here the re-indented line is top-level so it starts with spaces.

I, for one, am -1 on tabs but that's not a battle worth fighting probably. However, we can probably come up with a smarter test-check-code. If we're moving towards clang-format, it seems that the test-code should be checking that and not arbitrarily for spaces at the start of a line.

durin42 added a comment.Oct 14 2017, 3:07 AM
In D1028#17513, @yuja wrote:

Seemed fine, but test-check-code.t says no.

+  mercurial/mpatch.c:268:
+   >             ssize_t start, ssize_t end)
+   don't use spaces to indent
+  mercurial/mpatch.h:24:
+   >             ssize_t start, ssize_t end);
+   don't use spaces to indent
In D1028#17513, @yuja wrote:

Seemed fine, but test-check-code.t says no.

+  mercurial/mpatch.c:268:
+   >             ssize_t start, ssize_t end)
+   don't use spaces to indent
+  mercurial/mpatch.h:24:
+   >             ssize_t start, ssize_t end);
+   don't use spaces to indent

Ah. Okay. Since everyone at the sprint seemed loosely in favor of clang-format, how about this: I'll get the remaining trivial fixes from clang-format in, then land a test-check-clang-format.t with an extensive blacklist, then remove the check-code rules that are offended by clang-format as the series exposes them. That way we own less checking, and the formatting is done by a robot.

That work?

durin42 added a comment.Oct 14 2017, 3:09 AM
In D1028#17622, @ryanmce wrote:

I, for one, am -1 on tabs but that's not a battle worth fighting probably. However, we can probably come up with a smarter test-check-code. If we're moving towards clang-format, it seems that the test-code should be checking that and not arbitrarily for spaces at the start of a line.

I'm going to forge ahead with the coding style we nominally have in the codebase today, and we can consider changes once we're done with the initial move to the formatter. Sound good?

(I don't feel especially strongly, though my personal preference is still tabs-for-indentation spaces-for-alignment, which clang-format happily can do. I just have better things to argue about, which is why clang-format is great, especially if you get editor integration set up.)

durin42 updated this revision to Diff 2823.Oct 16 2017, 11:53 AM
Closed by commit rHGbd177139e635: mpatch: reformat function prototypes with clang-format (authored by durin42). · Explain WhyOct 16 2017, 9:46 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/mpatch.h (10 lines)
Mmercurial/mpatch.c (12 lines)

Diff 2823

mercurial/mpatch.h

#ifndef _HG_MPATCH_H_ #ifndef _HG_MPATCH_H_
#define _HG_MPATCH_H_ #define _HG_MPATCH_H_
#define MPATCH_ERR_NO_MEM -3 #define MPATCH_ERR_NO_MEM -3
#define MPATCH_ERR_CANNOT_BE_DECODED -2 #define MPATCH_ERR_CANNOT_BE_DECODED -2
#define MPATCH_ERR_INVALID_PATCH -1 #define MPATCH_ERR_INVALID_PATCH -1
struct mpatch_frag { struct mpatch_frag {
int start, end, len; int start, end, len;
const char *data; const char *data;
}; };
struct mpatch_flist { struct mpatch_flist {
struct mpatch_frag *base, *head, *tail; struct mpatch_frag *base, *head, *tail;
}; };
int mpatch_decode(const char *bin, ssize_t len, struct mpatch_flist** res); int mpatch_decode(const char *bin, ssize_t len, struct mpatch_flist **res);
ssize_t mpatch_calcsize(ssize_t len, struct mpatch_flist *l); ssize_t mpatch_calcsize(ssize_t len, struct mpatch_flist *l);
void mpatch_lfree(struct mpatch_flist *a); void mpatch_lfree(struct mpatch_flist *a);
int mpatch_apply(char *buf, const char *orig, ssize_t len, int mpatch_apply(char *buf, const char *orig, ssize_t len,
struct mpatch_flist *l); struct mpatch_flist *l);
struct mpatch_flist *mpatch_fold(void *bins, struct mpatch_flist *
struct mpatch_flist* (*get_next_item)(void*, ssize_t), mpatch_fold(void *bins, struct mpatch_flist *(*get_next_item)(void *, ssize_t),
ssize_t start, ssize_t end); ssize_t start, ssize_t end);
#endif #endif

mercurial/mpatch.c

{ {
return a->tail - a->head; return a->tail - a->head;
} }
/* move hunks in source that are less cut to dest, compensating /* move hunks in source that are less cut to dest, compensating
for changes in offset. the last hunk may be split if necessary. for changes in offset. the last hunk may be split if necessary.
*/ */
static int gather(struct mpatch_flist *dest, struct mpatch_flist *src, int cut, static int gather(struct mpatch_flist *dest, struct mpatch_flist *src, int cut,
int offset) int offset)
{ {
struct mpatch_frag *d = dest->tail, *s = src->head; struct mpatch_frag *d = dest->tail, *s = src->head;
int postend, c, l; int postend, c, l;
while (s != src->tail) { while (s != src->tail) {
if (s->start + offset >= cut) if (s->start + offset >= cut)
break; /* we've gone far enough */ break; /* we've gone far enough */
src->head = s; src->head = s;
return offset; return offset;
} }
/* combine hunk lists a and b, while adjusting b for offset changes in a/ /* combine hunk lists a and b, while adjusting b for offset changes in a/
this deletes a and b and returns the resultant list. */ this deletes a and b and returns the resultant list. */
static struct mpatch_flist *combine(struct mpatch_flist *a, static struct mpatch_flist *combine(struct mpatch_flist *a,
struct mpatch_flist *b) struct mpatch_flist *b)
{ {
struct mpatch_flist *c = NULL; struct mpatch_flist *c = NULL;
struct mpatch_frag *bh, *ct; struct mpatch_frag *bh, *ct;
int offset = 0, post; int offset = 0, post;
if (a && b) if (a && b)
c = lalloc((lsize(a) + lsize(b)) * 2); c = lalloc((lsize(a) + lsize(b)) * 2);
f++; f++;
} }
outlen += len - last; outlen += len - last;
return outlen; return outlen;
} }
int mpatch_apply(char *buf, const char *orig, ssize_t len, int mpatch_apply(char *buf, const char *orig, ssize_t len,
struct mpatch_flist *l) struct mpatch_flist *l)
{ {
struct mpatch_frag *f = l->head; struct mpatch_frag *f = l->head;
int last = 0; int last = 0;
char *p = buf; char *p = buf;
while (f != l->tail) { while (f != l->tail) {
if (f->start < last || f->end > len) { if (f->start < last || f->end > len) {
return MPATCH_ERR_INVALID_PATCH; return MPATCH_ERR_INVALID_PATCH;
} }
memcpy(p, orig + last, f->start - last); memcpy(p, orig + last, f->start - last);
p += f->start - last; p += f->start - last;
memcpy(p, f->data, f->len); memcpy(p, f->data, f->len);
last = f->end; last = f->end;
p += f->len; p += f->len;
f++; f++;
} }
memcpy(p, orig + last, len - last); memcpy(p, orig + last, len - last);
return 0; return 0;
} }
/* recursively generate a patch of all bins between start and end */ /* recursively generate a patch of all bins between start and end */
struct mpatch_flist *mpatch_fold(void *bins, struct mpatch_flist *
struct mpatch_flist* (*get_next_item)(void*, ssize_t), mpatch_fold(void *bins, struct mpatch_flist *(*get_next_item)(void *, ssize_t),
ssize_t start, ssize_t end) ssize_t start, ssize_t end)
{ {
ssize_t len; ssize_t len;
if (start + 1 == end) { if (start + 1 == end) {
/* trivial case, output a decoded list */ /* trivial case, output a decoded list */
return get_next_item(bins, start); return get_next_item(bins, start);
} }
/* divide and conquer, memory management is elsewhere */ /* divide and conquer, memory management is elsewhere */
len = (end - start) / 2; len = (end - start) / 2;
return combine(mpatch_fold(bins, get_next_item, start, start + len), return combine(mpatch_fold(bins, get_next_item, start, start + len),
mpatch_fold(bins, get_next_item, start + len, end)); mpatch_fold(bins, get_next_item, start + len, end));
} }