From: Jeff King <peff@peff.net>
To: Johannes Sixt <j6t@kdbg.org>
Cc: "brian m. carlson" <sandals@crustytoothpaste.net>,
git@vger.kernel.org, Elijah Newren <newren@gmail.com>,
Junio C Hamano <gitster@pobox.com>,
Stefan Beller <sbeller@google.com>
Subject: Re: [PATCH v2 0/8] object_id part 4
Date: Sun, 19 Jun 2016 05:24:48 -0400 [thread overview]
Message-ID: <20160619092448.GA12221@sigill.intra.peff.net> (raw)
In-Reply-To: <57665CC6.6070208@kdbg.org>
On Sun, Jun 19, 2016 at 10:50:14AM +0200, Johannes Sixt wrote:
> Am 19.06.2016 um 00:13 schrieb brian m. carlson:
> > * Adjust the Coccinelle patches to transform plain structs before
> > pointers to structs to avoid misconversions. This addresses the issue
> > that Peff caught originally.
>
> To avoid future mistakes, can you write down how "transform plain structs
> before pointers to structs" looks like? Is it a particular order of
> Coccinelle rules? Which part of the interdiff between the previous round and
> this round makes the difference?
Yeah, I'd also like a better understanding of what went wrong in the
original (just to be able to better evaluate the tool).
Also, for the record, the issue was noticed by Johannes originally, not
me, as indicated above (I just found a similar case after seeing his
report).
> On a tangent, I wondered recently, why we need oidcpy() and oidclr(). After
> all, in place of, e.g.,
>
> oidcpy(&pair->two->oid, &p->oid);
> oidclr(&one->oid);
>
> we can write
>
> pair->two->oid = p->oid;
> one->oid = null_oid;
>
> Is there a particular reason *not* to make this transition? I find the
> latter less cluttered with equal clarity.
I think traditionally we've avoided struct assignment because some
ancient compilers didn't do it. But it's in C89, and I suspect it's
crept into the code base anyway over the years without anyone
complaining.
-Peff
next prev parent reply other threads:[~2016-06-19 9:24 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-18 22:13 [PATCH v2 0/8] object_id part 4 brian m. carlson
2016-06-18 22:14 ` [PATCH v2 1/8] Add basic Coccinelle transforms brian m. carlson
2016-06-18 22:14 ` [PATCH v2 2/8] Apply object_id Coccinelle transformations brian m. carlson
2016-06-21 21:36 ` Junio C Hamano
2016-06-18 22:14 ` [PATCH v2 3/8] Convert struct diff_filespec to struct object_id brian m. carlson
2016-06-21 22:22 ` Junio C Hamano
2016-06-24 15:27 ` brian m. carlson
2016-06-18 22:14 ` [PATCH v2 4/8] Rename struct diff_filespec's sha1_valid member brian m. carlson
2016-06-18 22:14 ` [PATCH v2 5/8] merge-recursive: convert struct stage_data to use object_id brian m. carlson
2016-06-18 22:14 ` [PATCH v2 6/8] merge-recursive: convert struct merge_file_info to object_id brian m. carlson
2016-06-18 22:14 ` [PATCH v2 7/8] merge-recursive: convert leaf functions to use struct object_id brian m. carlson
2016-06-18 22:14 ` [PATCH v2 8/8] merge-recursive: convert merge_recursive_generic to object_id brian m. carlson
2016-06-19 8:50 ` [PATCH v2 0/8] object_id part 4 Johannes Sixt
2016-06-19 9:24 ` Jeff King [this message]
2016-06-19 17:25 ` brian m. carlson
2016-06-20 7:01 ` Johannes Schindelin
2016-06-20 10:05 ` Jeff King
2016-06-20 15:53 ` Junio C Hamano
2016-06-21 21:22 ` Junio C Hamano
2016-06-22 18:44 ` brian m. carlson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160619092448.GA12221@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=newren@gmail.com \
--cc=sandals@crustytoothpaste.net \
--cc=sbeller@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).