On Wed, 2016-06-22 at 02:57 -0400, Frediano Ziglio wrote:
> >
> >
> > There aren't many users of spice-server, and the required fix is pretty
> > small
> > here, but I don't think we should break API for this unless there's a really
> > good reason coming in the following patches...
> >
> > Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> >
> Yes, that's why I post as a separate patch.
>
> On the other side I think this code
>
> spice_server_char_device_recognized_subtypes()[0] = "new subtype";
>
> should not work.
absolutely agree
>
> One option to not break API would be to use an explicit cast like
>
> static const char *const
> spice_server_char_device_recognized_subtypes_list[]
> ...
>
> SPICE_GNUC_VISIBLE const char**
> spice_server_char_device_recognized_subtypes(void)
> {
> return (const char **) spice_server_char_device_recognized_subtypes_list;
> }
>
> This will probably cause the above previous code to compile correctly
> but give SIGSEGV as is changing a constant (usually for security reasons
> code is compiled with relro linking option which made these arrays
> constant in memory).
Personally, I think this is preferable. qemu does not change this returned
array, so it would not currently cause a crash in qemu. But if they changed
their behavior to modify the array, it would potentially introduce a crash,
which is fine since they're doing something illegal. I'd like to add a FIXME
comment somewhere indicating that we should change this interface at our next
API break.
>
> Frediano
>
>
> >
> >
> > On Mon, 2016-06-20 at 10:15 +0100, Frediano Ziglio wrote:
> > >
> > > Users should not change the list of supported subtypes.
> > > This could break the API but not the ABI.
> > >
> > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > > ---
> > > server/reds.c | 4 ++--
> > > server/spice-char.h | 2 +-
> > > 2 files changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/server/reds.c b/server/reds.c
> > > index b2438f6..3f33c32 100644
> > > --- a/server/reds.c
> > > +++ b/server/reds.c
> > > @@ -3150,7 +3150,7 @@ SPICE_GNUC_VISIBLE void
> > > spice_server_char_device_wakeup(SpiceCharDeviceInstance*
> > > #define SUBTYPE_USBREDIR "usbredir"
> > > #define SUBTYPE_PORT "port"
> > >
> > > -static const char *spice_server_char_device_recognized_subtypes_list[] =
> > > {
> > > +static const char *const
> > > spice_server_char_device_recognized_subtypes_list[]
> > > = {
> > > SUBTYPE_VDAGENT,
> > > #ifdef USE_SMARTCARD
> > > SUBTYPE_SMARTCARD,
> > > @@ -3159,7 +3159,7 @@ static const char
> > > *spice_server_char_device_recognized_subtypes_list[] = {
> > > NULL,
> > > };
> > >
> > > -SPICE_GNUC_VISIBLE const char**
> > > spice_server_char_device_recognized_subtypes(void)
> > > +SPICE_GNUC_VISIBLE const char* const*
> > > spice_server_char_device_recognized_subtypes(void)
> > > {
> > > return spice_server_char_device_recognized_subtypes_list;
> > > }
> > > diff --git a/server/spice-char.h b/server/spice-char.h
> > > index efd685d..e0348c2 100644
> > > --- a/server/spice-char.h
> > > +++ b/server/spice-char.h
> > > @@ -56,6 +56,6 @@ struct SpiceCharDeviceInstance {
> > >
> > > void spice_server_char_device_wakeup(SpiceCharDeviceInstance *sin);
> > > void spice_server_port_event(SpiceCharDeviceInstance *char_device,
> > > uint8_t
> > > event);
> > > -const char** spice_server_char_device_recognized_subtypes(void);
> > > +const char* const* spice_server_char_device_recognized_subtypes(void);
> > >
> > > #endif /* SPICE_CHAR_H_ */
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel