Re: [PATCH XEN v5 13/23] tools: Refactor foreign memory mapping into libxenforeignmemory

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 27/11/15 17:18, Ian Campbell wrote:
> On Fri, 2015-11-13 at 15:27 +0000, Ian Jackson wrote:
>> Ian Campbell writes ("Re: [PATCH XEN v5 13/23] tools: Refactor foreign
>> memory mapping into libxenforeignmemory"):
>>> On Wed, 2015-11-11 at 15:13 +0000, Ian Jackson wrote:
>>> Are you suggesting:
>>>
>>> void *xenforeignmemory_map(xenforeignmemory_handle *fmem, uint32_t dom,
>>>                            int prot, const xen_pfn_t *arr,
>>>                            int err_out[num],
>>>                            unsigned int num);
>>>
>>> Is that (a var sized array based on another param) really allowed?
>> Yes, if the array size precedes the array.
> Sadly:
>
>     for i in include/xenforeignmemory.h; do \
>         gcc -x c -ansi -Wall -Werror -pedantic -I/local/scratch/ianc/devel/libxenctrl-split/xen.git/tools/libs/foreignmemory/../../../tools/include \
>               -S -o /dev/null $i || exit 1; \
>         echo $i; \
>     done >headers.chk.new
>     include/xenforeignmemory.h:142:28: error: ISO C90 forbids variable length array ‘arr’ [-Werror=vla]
>                                 const xen_pfn_t *arr[pages], int err[pages]);
>                                 ^
>     include/xenforeignmemory.h:142:28: error: ISO C90 forbids variable length array ‘err’ [-Werror=vla]
>     cc1: all warnings being treated as errors
>     /local/scratch/ianc/devel/libxenctrl-split/xen.git/tools/libs/foreignmemory/../../../tools/Rules.mk:200: recipe for target 'headers.chk' failed
>
> It's -ansi (AKA -std=c90) together with -pedantic which does it. -std=c99
> is happy with it (including with -pedantic).
>
> The original motivation for this check was [0]:
>     As part of the tidyup, we should choose a particular C standard (89,
>     probably) and ensure that the API/ABI complies with `gcc -std=c$VER
>     -pedantic`.  This will help to provide a consistent API on other
>     platforms (I seem to recall an effort to port libvchan to windows.)
>
> It's not clear to me how much the "89 probably" was influenced by the "on
> other platforms ... windows" bit (maybe Andy or Paul has an opinion).

I was under the impression that MSVC still doesn't have C99 support.

According to wikipedia, VS2013 gained some C99, and 2015 "achieved a
level of the C99 standard compliance in the library that is similar to
other C compilers".

According to MSDN about Visual C++:

C99 Conformance: Visual Studio 2015 fully implements the C99 Standard
Library, with the exception of any library features that depend on
compiler features not yet supported by the Visual C++ compiler (for
example, <tgmath.h> is not implemented).

which is wonderfully circular.

According to
https://msdn.microsoft.com/en-us/library/zb1574zs%28v=vs.140%29.aspx
which appears to be the documentation, VLAs are still not supported,
even in VS2015.

>
> C99 was 16 years ago now, I'm struggling to think of a reason not to move
> the baseline for tools stuff at least to that.
>
> https://en.wikipedia.org/wiki/Visual_C%2B%2B might be one such reason I
> suppose, although I'm not convinced a libvchan port to Windows, even if not
> entirely hypothetical, would be using any of xen.git/tools/libs/* rather
> than the equivalent frameworks provided by the Windows PV drivers.

It would be nice to at least be able to use the same header files, for
ease of porting userspace software.

In this case, VLAs are just being used as an aid for the compiler to
spot errors.  It doesn't change the API/ABI, and could be #ifdef'd
around, if we care both for using C99 in general, and Windows support.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel