Re: [PATCH 3/5] programs/services: Break services into Loader groups

[Sebastian Lackner]

On 09.08.2016 18:40, Aric Stewart wrote:
> Sort the groups by the service's tag to start in order
>
> Signed-off-by: Aric Stewart <aric(a)codeweavers.com>
> ---
>  programs/services/services.c | 101 ++++++++++++++++++++++++++++++++-----------
>  1 file changed, 75 insertions(+), 26 deletions(-)

I believe this can be solved a bit easier. Why not just a sort function callback
which sorts for both group and tag?

[PATCH 3/5] programs/services: Break services into Loader groups

[Aric Stewart]

On 8/11/16 12:38 AM, Sebastian Lackner wrote:
> On 09.08.2016 18:40, Aric Stewart wrote:
> > Sort the groups by the service's tag to start in order
> >
> > Signed-off-by: Aric Stewart <aric(a)codeweavers.com>
> > ---
> >  programs/services/services.c | 101 ++++++++++++++++++++++++++++++++-----------
> >  1 file changed, 75 insertions(+), 26 deletions(-)
>
> I believe this can be solved a bit easier. Why not just a sort function callback
> which sorts for both group and tag?

I feel like that is what I am doing. Part of the reason I am breaking each group into a separate set it that we will be wanting to start a single winedevice process for each group and that felt like an easier way to organize it.

Alternate implementations are welcome.

-aric

[PATCH 3/5] programs/services: Break services into Loader groups

[Sebastian Lackner]

On 11.08.2016 14:04, Aric Stewart wrote:
> On 8/11/16 12:38 AM, Sebastian Lackner wrote:
> > On 09.08.2016 18:40, Aric Stewart wrote:
> > > Sort the groups by the service's tag to start in order
> > >
> > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> --- 
> > > programs/services/services.c | 101
> > > ++++++++++++++++++++++++++++++++----------- 1 file changed, 75
> > > insertions(+), 26 deletions(-)
> >
> > I believe this can be solved a bit easier. Why not just a sort
> > function callback which sorts for both group and tag?
>
> I feel like that is what I am doing. Part of the reason I am breaking
> each group into a separate set it that we will be wanting to start a
> single winedevice process for each group and that felt like an easier
> way to organize it.
>
> Alternate implementations are welcome.
>
> -aric

Well, one winedevice process might not always be sufficient. Wine also
supports loading 32-bit drivers in a 64-bit wineprefix, so we might need
at least two per group (default and wow64). Because of this issue, it
seems easier to search for a matching winedevice instance during process
startup.

In this patch, I would suggest to implement only the sorting itself, and
then later let service_start_process() deal with the search for a running
process of the same group / architecture / (probably other conditions).
This would reduce the patch to a call of qsort() with a callback which
sorts for both group and tag. I hope it is clear what I mean, if not I
can also help you updating these parts.

Regards,
Sebastian

[PATCH 3/5] programs/services: Break services into Loader groups

[Aric Stewart]

On 8/11/16 7:19 AM, Sebastian Lackner wrote:
> On 11.08.2016 14:04, Aric Stewart wrote:
> > On 8/11/16 12:38 AM, Sebastian Lackner wrote:
> > > On 09.08.2016 18:40, Aric Stewart wrote:
> > > > Sort the groups by the service's tag to start in order
> > > >
> > > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> --- 
> > > > programs/services/services.c | 101
> > > > ++++++++++++++++++++++++++++++++----------- 1 file changed, 75
> > > > insertions(+), 26 deletions(-)
> > >
> > > I believe this can be solved a bit easier. Why not just a sort
> > > function callback which sorts for both group and tag?
> >
> > I feel like that is what I am doing. Part of the reason I am breaking
> > each group into a separate set it that we will be wanting to start a
> > single winedevice process for each group and that felt like an easier
> > way to organize it.
> >
> > Alternate implementations are welcome.
> >
> > -aric
>
> Well, one winedevice process might not always be sufficient. Wine also
> supports loading 32-bit drivers in a 64-bit wineprefix, so we might need
> at least two per group (default and wow64). Because of this issue, it
> seems easier to search for a matching winedevice instance during process
> startup.
>
> In this patch, I would suggest to implement only the sorting itself, and
> then later let service_start_process() deal with the search for a running
> process of the same group / architecture / (probably other conditions).
> This would reduce the patch to a call of qsort() with a callback which
> sorts for both group and tag. I hope it is clear what I mean, if not I
> can also help you updating these parts.

I know little to nothing about 64-bit and its effect here. So that is new. 

If you have a clear idea on how you would like this to work then please feel free, that would be most useful.
Unfortunately I am unable to dedicate my full time to this right now. (Too many other work projects demanding attention) 

I would really like to get this functionality in (Multiple devices in the same process) so that I can shift to my Plug and play and HID work. 

Otherwise I can circle back to this later today or a little tomorrow. You do have all my work here now submitted so you can see where I am going and what I need. 

thanks,
-aric