Re: [PATCH 5/7] winemac.drv: Add macdrv_get_monitors() helper.

[Zhiyi Zhang]

On 4/23/19 12:27 PM, Ken Thomases wrote:
> On Apr 22, 2019, at 7:13 AM, Zhiyi Zhang  wrote:
> > Signed-off-by: Zhiyi Zhang
> >
> > dlls/winemac.drv/cocoa_display.m | 108 +++++++++++++++++++++++++++++++++++++++
> > dlls/winemac.drv/macdrv_cocoa.h  |  18 +++++++
> > 2 files changed, 126 insertions(+)
> >
> > diff --git a/dlls/winemac.drv/cocoa_display.m b/dlls/winemac.drv/cocoa_display.m
> > index d95cda59c9..0005e2ac37 100644
> > --- a/dlls/winemac.drv/cocoa_display.m
> > +++ b/dlls/winemac.drv/cocoa_display.m
> > @@ -343,3 +343,111 @@ void macdrv_free_adapters(struct macdrv_adapter* adapters)
> > {
> >     free(adapters);
> > }
> > -
> >
> > +/***********************************************************************
> > -
> > -
> >          macdrv_get_monitors
> > -
> > -
> > -
> > - Get a list of monitors under adapter_id. The first monitor is primary if adapter is primary.
> > -
> > - Call macdrv_free_monitors() when you are done using the data.
> > -
> > -
> > -
> > - Return -1 on failure with parameters unchanged.
> > - */
> >
> > +int macdrv_get_monitors(uint32_t adapter_id, struct macdrv_monitor** new_monitors, int* count)
> > +{
> > - struct macdrv_monitor* monitors = NULL;
> > - struct macdrv_monitor* realloc_monitors;
> > - struct macdrv_display* displays = NULL;
> > - CGDirectDisplayID display_ids[16];
> > - uint32_t display_id_count;
> > - int primary_index = 0;
> > - int monitor_count = 0;
> > - int display_count;
> > - int capacity;
> > - int i, j;
> > - int ret = -1;
> > -
> > - /* 2 should be enough for most cases */
> > - capacity = 2;
> > - monitors = calloc(capacity, sizeof(*monitors));
> > - if (!monitors)
> > -
> >    return -1;
> > -
> > - /* Report an inactive monitor */
> > - if (!CGDisplayIsActive(adapter_id) && !CGDisplayIsInMirrorSet(adapter_id))
> > - {
> > -
> >    strcpy(monitors[monitor_count].name, "Generic Non-PnP Monitor");
> > -
> >    monitors[monitor_count].state_flags = DISPLAY_DEVICE_ATTACHED;
> > -
> >    monitor_count++;
> > - }
> > - /* Report active monitor and mirrored monitors in the same mirroring set */
> > - else
> > - {
> > -
> >    if (CGGetOnlineDisplayList(sizeof(display_ids) / sizeof(display_ids[0]), display_ids, &display_id_count)
> > -
> >        != kCGErrorSuccess)
> > -
> >        goto fail;
> > -
> > -
> >    if (macdrv_get_displays(&displays, &display_count))
> > -
> >        goto fail;
> > -
> > -
> >    for (i = 0; i < display_id_count; i++)
> > -
> >    {
> > -
> >        if (display_ids[i] != adapter_id && CGDisplayMirrorsDisplay(display_ids[i]) != adapter_id)
> > -
> >            continue;
> > -
> > -
> >        /* Find and fill in monitor info */
> > -
> >        for (j = 0; j < display_count; j++)
> > -
> >        {
> > -
> >            if (displays[i].displayID == display_ids[j]
> > -
> >                || CGDisplayMirrorsDisplay(display_ids[i]) == displays[j].displayID)
> > -
> >            {
> > -
> >                /* Allocate more space if needed */
> > -
> >                if (monitor_count >= capacity)
> > -
> >                {
> > -
> >                    capacity *= 2;
> > -
> >                    realloc_monitors = realloc(monitors, sizeof(*monitors) * capacity);
> > -
> >                    if (!realloc_monitors)
> > -
> >                        goto fail;
> > -
> >                    monitors = realloc_monitors;
> > -
> >                }
> > -
> > -
> >                if (!primary_index && CGDisplayIsMain(display_ids[j]))
> > -
> >                    primary_index = monitor_count;
> > -
> > -
> >                strcpy(monitors[monitor_count].name, "Generic Non-PnP Monitor");
> > -
> >                monitors[monitor_count].state_flags = DISPLAY_DEVICE_ATTACHED | DISPLAY_DEVICE_ACTIVE;
> > -
> >                monitors[monitor_count].rc_monitor = displays[j].frame;
> > -
> >                monitors[monitor_count].rc_work = displays[j].work_frame;
> > -
> >                monitor_count++;
> > -
> >            }
> > -
> >        }
> > -
> > -
> >        /* Make sure the first monitor on primary adapter is primary */
> > -
> >        if (primary_index)
> > -
> >        {
> > -
> >            struct macdrv_monitor tmp;
> > -
> >            tmp = monitors[0];
> > -
> >            monitors[0] = monitors[primary_index];
> > -
> >            monitors[primary_index] = tmp;
> > -
> >        }
>
> This swap is inside the "i" loop.  That means that you could keep swapping those two elements an arbitrary number of times.

Thanks. I must have messed it up during refactoring.
> > -
> >    }
> > - }
> > -
> > - *new_monitors = monitors;
> > - *count = monitor_count;
> > - ret = 0;
> >
> > +fail:
> > - macdrv_free_displays(displays);
> > - if (ret)
> > -
> >    free(monitors);
> > - return ret;
> >
> > +}