Re: [PATCH 3/8] Constify spice_server_char_device_recognized_subtypes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> 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.

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).

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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]