On Mon, 9 Oct 2017 12:54:03 +0200 Halil Pasic <pa...@linux.vnet.ibm.com> wrote:
> On 10/09/2017 10:20 AM, Thomas Huth wrote: > > On 04.10.2017 17:41, Halil Pasic wrote: > >> +/* IO instructions conclude according this */ > >> +typedef struct IOInstEnding { > >> + /* > >> + * General semantic of cc codes of IO instructions is (brief): > >> + * 0 -- produced expected result > >> + * 1 -- status conditions were present or produced alternate > >> result > >> + * 2 -- ineffective, because busy with previously initiated > >> function > >> + * 3 -- ineffective, not operational > >> + */ > >> + int cc; > >> +} IOInstEnding; > > > > Why do you need a struct for this? Do you plan to extend it later? If > > so, I think you should mention that in the patch description. If not, > > please use a named enum or a "typedef unsigned int IOInstEnding" instead. > > > > Thomas > > We may, we may not. In the previous version we also had to support > do end a certain instruction with an addressing exception, but this > is going away in patch #3. Honestly I don't expect this being extended. > > I have other reasons for the struct. Type safety and clear semantics, > and frankly at least for s390 and linux I don't see any downsides given > what is written in the "zSeries ELF Application Binary Interface Supplement". > Can you please explain to me what is the problem with using this struct, and > what is the benefit switching to a unsigned int? Honestly, I fail to see the benefit of using a struct here... it's just a condition code, and while adding a comment what the various codes mean for I/O instructions is a good idea, I think having to use a IOInstEnding struct just renders the code less readable. [I haven't had time to look at the rest of the patches yet.]