Re: [Qemu-devel] [PATCH RFC] scripts/update-linux-headers.sh: pull virtio hdrs

[Peter Maydell]

On 9 February 2015 at 19:56, Michael S. Tsirkin <address@hidden> wrote:
> +rm -rf "$output/standard-headers/linux"
> +mkdir -p "$output/standard-headers/linux"
> +for f in $tmpdir/include/linux/virtio*h; do
> +    header=$(expr "$f" : '.*/\(.*\)');
> +    sed -e 's/__u\([0-9][0-9]*\)/uint\1_t/g' \
> +        -e 's/linux\/types/inttypes/' \
> +        -e 's/__bitwise__//' \
> +        "$tmpdir/include/linux/$header" > \
> +        "$output/standard-headers/linux/$header";
> +done

This doesn't seem to be doing anything to fix up
the '__attribute__((packed))' annotations.
Presumably you're intending to put standard-headers/
on the include path? It would probably be better to
not make the headers be in "linux/" in that case,
since it would mean confusion/clashes for what
"linux/virtio_net.h" etc mean -- are they the QEMU
sanitized versions or the host OS's? (Having them
in linux/ also makes code review harder since it breaks
the current rule of thumb that is "no include of
linux/anything in code that's not Linux-host-specific".)

You need to strip out all the #include <linux/something.h>
from these headers, otherwise this won't build on non
Linux hosts. Then you need to add in whatever the
equivalent is to get the defines/types those includes
were providing (for instance our virtio-net.h does a
simple #define of ETH_ALEN). You probably want to make
the update script fail if there's an include it's not
expecting to deal with, otherwise you're likely to
end up with a set of headers that seem OK on Linux
but fail when tested on other OSes -- better to fail
early and for the person trying to do the header update
than to end up with a change that won't pass my build
tests and gets bounced.

All that makes it seem to me like it's more trouble
than it's worth compared to doing a one-time manual
import.

-- PMM