On Tue, Apr 25, 2017 at 09:45:53AM +0200, Jan Tulak wrote: > On Tue, Apr 25, 2017 at 5:04 AM, Luis R. Rodriguez <mcgrof@xxxxxxxxxx> wrote: > > On Sun, Apr 23, 2017 at 08:54:55PM +0200, Jan Tulak wrote: > >> opts[MAX_OPTS] = { > >> +#define OPT_B 0 > > > > I find these defines blinding on reading the struct declaration, we could > > instead just stuff them into an enum, which would also enable us to shift all > > the documentation into the header of the enum using whatever doc format we use. > > The grouping is here to keep the style as it is with suboptions > index/string. However, all these defines (OPT_X and X_FOO) are > separated and moved into a single place later on in one patch anyway. > I just didn't include that patch in this set as I had to make the line > somewhere if I want to make multiple smaller and easier to review and > merge sets rather than the one big monster as before. Makes sense. > There is one point I'm not sure about, though. We have to restart the > counting for suboptions, like this: > > enum subopts { > B_size = 0, > B_LOG, > D_AGCOUNT = 0, > D_..., > L_xxx = 0, > L_..., > } > > I didn't found anywhere if it is a good practice or not. But again, I > just submitted it as it was with a note in my TODOs to find out more > out it and update the patch changing it later on. Ugh yeah no that would be pretty terrible, how about something like: enum subopt_zero_idx { SUBOPT_ZERO_A = 0, SUBOPT_ZERO_B, SUBOPT_ZERO_C, }; enum subopt_one_idx { SUBOPT_ONE_A = 0, SUBOPT_ONE_B, }; enum subopt_two_idx { SUBOPT_TWO_A = 0, }; union subopt_idx { enum subopt_zero_idx subopt_zero; enum subopt_one_idx subopt_one; enum subopt_two_idx subopt_two; }; Demo: http://drvbp1.linux-foundation.org/~mcgrof/demos/demo-flexible-array-subopts.c Luis -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html