On Thu, Jun 19, 2014 at 05:17:40PM +0300, Andrei Errapart wrote: > First, thanks for accepting 3 of the patches! > > 19.06.2014 14:14, David Gibson kirjutas: > >Ugh, this is by far the ugliest of these workarounds, and the reason > >for it in the code is really non-obvious. I'll have to think about > >this some more. > > Good point. Kind of careless of me not to pay attention to that aspect. > > What about dropping the field "num_prereqs" altogether? It forces someone > looping over the field "prereq" to think about the end condition and > comparision with NULL (or nullptr) is an obvious answer. A diff is at the > end of the letter. Hm, that might work. I'll think on it. > >> * Initialize check->inprogress, too. > > > >Why..? > > Stumbled over that one - the value of inprogress was in some cases > initialized to "true" when compiled with MSVC. > > According to the C99 draft > (http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf, pages 137,138), > in static storage all fields are initialized to zero, null, 0. Exactly.. > According to MSDN (http://msdn.microsoft.com/en-us/library/81k8cwsz.aspx), > initilization is required: "If initializer-list has fewer values than an > aggregate type, the remaining members or elements of the aggregate type are > initialized to 0. The initial value of an automatic identifier not > explicitly initialized is undefined." Right, an automatic identifier, which is to say a local variable, not a global variable like this, > >I've applied patches 2, 3, and 4. But for future reference, please remember: > > > > * Patches should be inline, not attachments > > * Commit message and signed-off-by lines should not be indented > > * Commit messages need more details on the reason for the patch (see > > the changes I've made for examples). > > * Commit messages need a 1 line summary at the top (2, 3, and 4 have > > this, but 1 doesn't) > > * There should be a blank line between the 1-lite summary and the > > remainder of the commit message > > Thanks again. > > Is there an automated command to format the commit message per the > requirements? git format-patch should do it, or git send-email will format and send out the emails for you too. > diff --git a/checks.c b/checks.c > index 47eda65..fe90dfc 100644 > --- a/checks.c > +++ b/checks.c > @@ -54,12 +54,11 @@ struct check { > bool warn, error; > enum checkstatus status; > bool inprogress; > - int num_prereqs; > struct check **prereq; > }; > > #define CHECK_ENTRY(nm, tfn, nfn, pfn, d, w, e, ...) \ > - static struct check *nm##_prereqs[] = { __VA_ARGS__ }; \ > + static struct check *nm##_prereqs[] = { __VA_ARGS__, NULL }; \ > static struct check nm = { \ > .name = #nm, \ > .tree_fn = (tfn), \ > @@ -69,7 +68,6 @@ struct check { > .warn = (w), \ > .error = (e), \ > .status = UNCHECKED, \ > - .num_prereqs = ARRAY_SIZE(nm##_prereqs), \ > .prereq = nm##_prereqs, \ > }; > #define WARNING(nm, tfn, nfn, pfn, d, ...) \ > @@ -153,7 +151,7 @@ static bool run_check(struct check *c, struct node *dt) > > c->inprogress = true; > > - for (i = 0; i < c->num_prereqs; i++) { > + for (i = 0; c->prereq[i] != NULL; i++) { > struct check *prq = c->prereq[i]; > error = error || run_check(prq, dt); > if (prq->status != PASSED) { > @@ -192,7 +190,7 @@ static inline void check_always_fail(struct check *c, > struct node *dt) > { > FAIL(c, "always_fail check"); > } > -TREE_CHECK(always_fail, NULL); > +TREE_CHECK(always_fail, NULL, NULL); Why is the extra NULL necessary here (and similar places)? -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Attachment:
pgpHFaK86siX2.pgp
Description: PGP signature
