On Sat, Mar 12, 2016 at 12:35 AM, Emil Velikov <emil.l.veli...@gmail.com> wrote: > On 9 March 2016 at 18:36, Marek Olšák <mar...@gmail.com> wrote: >> On Wed, Mar 9, 2016 at 6:51 PM, Emil Velikov <emil.l.veli...@gmail.com> >> wrote: >>> On 9 March 2016 at 17:11, Marek Olšák <mar...@gmail.com> wrote: >>>> On Wed, Mar 9, 2016 at 4:31 PM, Emil Velikov <emil.l.veli...@gmail.com> >>>> wrote: >>>>> On 8 March 2016 at 22:29, Marek Olšák <mar...@gmail.com> wrote: >>>>> >>>>>> Actually, I don't see how the version number would make it any better >>>>>> for the structures, but returning the version number by >>>>>> QueryDeviceInfo would be useful for the caller to know what to expect >>>>>> if Mesa version < caller version. The sizes are still useful if Mesa >>>>>> version > caller version. >>>>>> >>>>> If any of this is an issue, then the whole DRI model just won't work ;-) >>>> >>>> The DRI extension versions only determine the number of function >>>> callbacks, not function parameters and return values. This interop >>>> thing is a lot more complicated than that, since it allows the "in" >>>> and "out" structures to grow, and different rules apply to each. >>> The fact that the DRI extension version is used only to determine the >>> presence of certain function, is implementation detail imho. >>> >>> If you look at the struct as a whole, it doesn't matter what the >>> contents (part the version field) are. For all one care there could be >>> none ? >>> >>>> Also, >>>> the implementation of DRI extensions allocates the structures, while >>>> in the interop the user allocates the structures. It's a totally >>>> different model. >>>> >>> True, it's a slightly different model. The following should still work ? >>> >>> Library A: allocates memory for the struct, and set the currently >>> maximum supported version >>> Library B: retrieves the 'empty' struct. Checks which version is lower >>> (self and the one in the struct) and only populates for up-to that >>> version. >>> >>>>> >>>>> I'm thinking that the following should work. Please let me know if I'm >>>>> loosing the plot. >>>>> >>>>> Caller sets the structure and sets version of the interface it >>>>> provides. Then callee first checks if it can work with the provider >>>>> version. then proceeds as planned. >>>> >>>> The callee must always proceed even if the version is too low or too >>>> high. >>> What do you mean with "proceed" here ? >>> >>> Are you saying that even if the callee can work with up-to v2, it >>> should somehow populate the v3 fields ? >>> Shouldn't it use the lowest version of the two and only handle those fields >>> ? >> >> A v1 callee can only read and write v1 fields. >> A v2 callee can only read and write v2 fields, but will only write v1 >> fields if the size doesn't include v2 fields. >> I could go on. >> >> The callee doesn't care about the caller's version at all. >> > With the last sentence removed, that's roughly what I was saying. > > The library that allocates the struct, sets the layout version ('size' > if you prefer). Then both libraries can read/write fields up-to the > smaller version (size) between the one already set in the struct and > the one they support.
Yeah, that's what the code does with the exception that v5 uses "interop_version" returned by QueryDeviceInfo instead of a size. > To make is more prominent one can explicitly > mark the version as const and add a small comment that only the > function which allocates the struct is responsible for setting the > variable. This last sentence also applies for 'size' fwiw. Well, the sizes are already function parameters, not structure members. > > Out of curiosity: did you draw some inspiration about using 'size' > based on some other interfaces ? I'm sorry but they serve no benefit, > plus the implementation requires some nasty code to work with them. "offset_after" isn't nasty. :) Yeah, I was inspired by other APIs I had seen. The sizes make even more sense when they are function parameters, because the caller can just do: (sizeof(in), &in, sizeof(out), &out) > >>> >>>> The caller knows the Mesa interop version from QueryDeviceInfo >>>> (defined in v4 and later) and knows what it should expect. Mesa only >>>> checks the sizes, but otherwise doesn't care about the caller's >>>> version. >>>> >>>>> Passing around multiple sizes is ugly and error prone to a point. One >>>>> gets the order wrong (swaps in_size and out_size) or just the same >>>>> sizeof(struct foo) in both places. >>>> >>>> The order of parameters in v5 is: in_size, *in, out_size, *out >>> >>>> If the implementation swaps in_size and out_size, it's no my problem. >>> No offence, but you realise that this sounds rather funny, right ? >>> In the context of designing interfaces, of course. >> >> It's not my problem for the same reason that swapping src and dst in >> memcpy is not glibc's problem. We have to draw the line somewhere. :) >> > Was talking about the sizes, not the in/out pointers. There is a > subtle difference with the possible side-falls and their > detection/prevention. We should catch such bugs with tests. Marek _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev