Re: [Qemu-devel] [PATCH v9 04/17] vnc: hoist allocation of VncBasicInfo to callers

[Gerd Hoffmann <kraxel@redhat.com>]

  Hi,

> > -static VncBasicInfo *vnc_basic_info_get(struct sockaddr_storage *sa,
> > -                                        socklen_t salen)
> > +static void vnc_basic_info_get(struct sockaddr_storage *sa,
> > +                               socklen_t salen,
> > +                               VncBasicInfo *info,
> > +                               Error **errp)

> The function no longer "gets info", it merely initializes it.  Rename it
> accordingly?  Gerd?

vnc_fill_basic_info maybe?

> Outside this patch's scope, but since I'm looking at the code already...
> 
> Here's the only caller of vnc_server_info_get():
> 
>     static void vnc_qmp_event(VncState *vs, QAPIEvent event)
>     {
>         VncServerInfo *si;
> 
>         if (!vs->info) {
>             return;
>         }
>         g_assert(vs->info->base);
> 
>         si = vnc_server_info_get(vs->vd);
>         if (!si) {
> --->        return;
>         }
> 
>         switch (event) {
>         case QAPI_EVENT_VNC_CONNECTED:
>             qapi_event_send_vnc_connected(si, vs->info->base, &error_abort);
>             break;
>         case QAPI_EVENT_VNC_INITIALIZED:
>             qapi_event_send_vnc_initialized(si, vs->info, &error_abort);
>             break;
>         case QAPI_EVENT_VNC_DISCONNECTED:
>             qapi_event_send_vnc_disconnected(si, vs->info, &error_abort);
>             break;
>         default:
>             break;
>         }
> 
>         qapi_free_VncServerInfo(si);
>     }
> 
> When vnc_server_info_get() fails, the event is dropped.  Why is that
> okay?  Failure seems unlikely, though.

Suggestions what else to do?  I don't wanna crash qemu by calling
qapi_event_send_vnc_* with a NULL pointer.  And, yes, it should be
highly unlikely so trying some more sophisticated error handling would
probably be dead code ...

cheers,
  Gerd