[Spice-devel] [spice-server] Add some NULL checks to spice_server_remove_interface()

[Christophe Fergeau]

On Tue, Jan 31, 2017 at 11:20:45AM -0500, Frediano Ziglio wrote:
> > 
> > On Mon, Jan 30, 2017 at 01:26:01PM -0500, Frediano Ziglio wrote:
> > > >      if (strcmp(interface->type, SPICE_INTERFACE_TABLET) == 0) {
> > > >          SpiceTabletInstance *tablet = SPICE_CONTAINEROF(sin,
> > > >          SpiceTabletInstance, base);
> > > > +        g_return_val_if_fail(tablet->st != NULL, -2);
> > > >          reds = spice_tablet_state_get_server(tablet->st);
> > > >          spice_info("remove SPICE_INTERFACE_TABLET");
> > > >          inputs_channel_detach_tablet(reds->inputs_channel, tablet);
> > > > @@ -3321,12 +3324,14 @@ SPICE_GNUC_VISIBLE int
> > > > spice_server_remove_interface(SpiceBaseInstance *sin)
> > > >          snd_detach_record(SPICE_CONTAINEROF(sin, SpiceRecordInstance,
> > > >          base));
> > > >      } else if (strcmp(interface->type, SPICE_INTERFACE_CHAR_DEVICE) ==
> > > >      0) {
> > > >          SpiceCharDeviceInstance *char_device = SPICE_CONTAINEROF(sin,
> > > >          SpiceCharDeviceInstance, base);
> > > > +        g_return_val_if_fail(char_device->st != NULL, -2);
> > > >          reds = red_char_device_get_server(char_device->st);
> > > >          spice_server_char_device_remove_interface(reds, sin);
> > > >      } else if (strcmp(interface->type, SPICE_INTERFACE_QXL) == 0) {
> > > >          QXLInstance *qxl;
> > > >  
> > > >          qxl = SPICE_CONTAINEROF(sin, QXLInstance, base);
> > > > +        g_return_val_if_fail(qxl->st != NULL, -2);
> > > >          reds = red_qxl_get_server(qxl->st);
> > > >          reds->qxl_instances = g_list_remove(reds->qxl_instances, qxl);
> > > >          red_qxl_destroy(qxl);
> > > 
> > > Why -2 ?
> > > 
> > Did not know whether to reuse -1, or to return a different error code.
> > I chose to return -2 as a different code, I don't mind reusing -1 if
> > that's preferred.
> > 
> > Christophe
> > 
> 
> I looked at server/spice-server.h and there's no documentation, just the function
> returns int and from code 0 for success and once -1 for error.
> So even 1 would be fine... no idea, we should add some documentation probably.
> Qemu seems to ignore the return beside when create displays.
> I would just return -1 as many C/POSIX functions returns -1 for error.
> But I think we are free.. perhaps -EINVAL or -ENODEV ? (in this case -1 would
> mean -EPERM, if we define like that).

I think I'll just stick with -1 for now :)

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170131/ab757efa/attachment-0001.sig>