On Thu, Jul 21, 2022 at 7:10 AM Michael Paquier <mich...@paquier.xyz> wrote: > > On Wed, Jul 20, 2022 at 02:32:44PM +0530, Amit Kapila wrote: > > On Thu, Jul 14, 2022 at 4:34 AM Peter Smith <smithpb2...@gmail.com> wrote: > >> By using a bitmask I think there is an implication that the flags can > >> be combined... > >> > >> Perhaps it is not a problem today, but later you may want more flags. e.g. > >> #define SUBSCRIPTION_REL_STATE_READY 0x02 /* READY relations */ > >> > >> then the bitmask idea falls apart because IIUC you have no intentions > >> to permit things like: > >> (SUBSCRIPTION_REL_STATE_NOT_READY | SUBSCRIPTION_REL_STATE_READY) > > > > I think this will be an invalid combination if caller ever used it. > > However, one might need to use a combination like > > (SUBSCRIPTION_REL_STATE_READY | SUBSCRIPTION_REL_STATE_DONE). For such > > cases, I feel the bitmask idea will be better. > > It feels unnatural to me to have a flag saying "not-ready" and one > saying "ready", while we could have a flag saying "ready" that can be > combined with a second flag to decide if the contents of srsubstate > should be matched or *not* matched with the states expected by the > caller. This could be extended to more state values, for example. > > I am not sure if we actually need this much as I have no idea if > future features would use it, so please take my suggestion lightly :) >
Yeah, it is not very clear to me either. I think this won't be difficult to change one or another way depending on future needs. At this stage, it appeared to me that bitmask is a better way to represent this information but if you and other feels using enum is a better idea then I am fine with that as well. -- With Regards, Amit Kapila.
