Re: [PATCH v4 1/4] s390x/css: Introduce an ESW struct

[Cornelia Huck]

On Fri, Jun 18 2021, Eric Farman <farman@linux.ibm.com> wrote:

> On Fri, 2021-06-18 at 14:46 +0200, Cornelia Huck wrote:
>> On Fri, Jun 18 2021, Cornelia Huck <cohuck@redhat.com> wrote:
>> 
>> > On Fri, Jun 18 2021, Eric Farman <farman@linux.ibm.com> wrote:
>> > > +/* format-0 extended-status word */
>> > > +typedef struct ESW {
>> > > +   uint32_t word0;
>> > > +   uint32_t erw;
>> > > +   uint64_t f_addr;     /* Zeros for other ESW formats */
>> > > +   uint32_t s_addr;     /* Zeros for other ESW formats */
>> > > +} QEMU_PACKED ESW;
>> > 
>> > Eww, this fails with mingw:
>> > https://gitlab.com/cohuck/qemu/-/jobs/1358335494
>> > In file included from /usr/i686-w64-mingw32/sys-
>> > root/mingw/include/winsock2.h:54,
>> >                  from /builds/cohuck/qemu/include/sysemu/os-
>> > win32.h:29,
>> >                  from /builds/cohuck/qemu/include/qemu/osdep.h:135,
>> >                  from ../hw/s390x/virtio-ccw-gpu.c:11:
>> > /builds/cohuck/qemu/include/hw/s390x/ioinst.h:131:13: error:
>> > expected ':', ',', ';', '}' or '__attribute__' before '.' token
>> >   131 |    uint32_t s_addr;     /* Zeros for other ESW formats */
>> >       |             ^~~~~~
>> 
>> It seems to be the name that is tripping it; if I rename it to
>> sec_addr
>> and the preceding field to fail_addr, the build passes.
>
> I was just wondering if it might have been the underscore directly, not
> that it was a single letter before the underscore. Weird.
>
>> 
>> Anyone know why that is? And if renaming is unavoidable, are
>> fail_addr
>> and sec_addr ok, or can we find better names?
>
> Since they're zero for Format-!0 ESWs, and regardless we don't fill
> them in anyway, could we just make them wordN and change the comment to
> give the descriptive name?
>
>  /* format-0 extended-status word */
>  typedef struct ESW {
> -   uint32_t
> word0;
> +   uint32_t word0;      /* subchannel logout for format 0 */
>     
> uint32_t erw;
> -   uint64_t f_addr;     /* Zeros for other ESW formats */
> -   uint32_t s_addr;     /* Zeros for other ESW formats */
> +   uint64_t
> word2;      /* failing-storage address for format 0 */
> +   uint32_t
> word4;      /* scondary-CCW address for format 0 */
>  } QEMU_PACKED ESW;
>

Yeah, that looks even better.

I can change that myself, will push out to test.