On 06/05/2015 11:45 AM, Jeff King wrote:
> On Fri, Jun 05, 2015 at 01:41:21AM +0000, brian m. carlson wrote:
>
>> However, with the object_id conversion, we run into a problem: casting
>> those unsigned char * values into struct object_id * values is not
>> allowed by the C standard. There are two possible solutions: copying;
>> and the just-do-it solution, where we cast and hope for the best.
>
> [...]
>
> But I'm not even sure that line of thinking is all that interesting.
> Even if we are violating some dark corner of the standard, this
> definitely falls into the "it's useful and works on all sane machines"
> category. We also do much worse things with struct-casting mmap'd data
> elsewhere (e.g., see the use of "struct pack_header"). It works fine in
> practice as long as you are careful about alignment and padding issues.
>
> So my vote would be to retain the cast. This is very low-level,
> performance-sensitive code. I did some very naive timings and didn't see
> any measurable change from your patch, but I also don't think we are
> seeing a real portability benefit to moving to the copy, so I'd prefer
> to keep the status quo.
I don't know that there would necessarily be problems, but I would worry
about code involving structure assignment. For example, suppose the
following snippet:
void f(struct object_id *oid)
{
struct object_id oid_copy = *oid;
/* ... */
}
The compiler is allowed to implement the copy using instructions that
rely on proper alignment. Such code would fail if oid is not properly
aligned.
Michael
--
Michael Haggerty
mhagger@xxxxxxxxxxxx
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html