Re: [Qemu-devel] [PATCH v2 2/8] s390x/css: IO instr handler ending control

[Cornelia Huck]

On Tue, 10 Oct 2017 12:28:35 +0200
Thomas Huth <address@hidden> wrote:

> On 09.10.2017 17:00, Halil Pasic wrote:
> > 
> > 
> > On 10/09/2017 01:07 PM, Thomas Huth wrote:  

> >> Then, in the follow up patches, you do something like this:
> >>
> >>    return (IOInstEnding){.cc = 0};
> >>
> >> ... and that just looks very, very ugly in my eyes. The more I look at  
> > 
> > Interesting, I found this quite expressive.  
> 
> C'mon, we're writing C code, not Java ;-)

Every time I read that construct, I die a little bit inside...

> Well, you already gave a description in your comment in the  struct
> IOInstEnding, so maybe something similar? Or maybe this could even be
> merged with the definitions for the SIGP status codes:
> 
> #define SIGP_CC_ORDER_CODE_ACCEPTED 0
> #define SIGP_CC_STATUS_STORED       1
> #define SIGP_CC_BUSY                2
> #define SIGP_CC_NOT_OPERATIONAL     3

I'd rather not reuse the definitions for a different instruction, even
if they are similar in semantics.

> > Sorry, I may be a bit to persistent on this one: I don't think it's
> > a huge difference, but I don't feel great about changing something to
> > what I think is (slightly) worse without being first convinced that
> > I was wrong.  
> 
> In the end, the code has to be accepted by the maintainers, so let's
> leave the decision up to them whether they like this typedef struct
> IOInstEnding or not...

Here's a strong 'do not like' from me... using an enum or define is
fine with me.