[Spice-devel] [spice-gtk 5/4] gtk-session: move var declaration to the top

[Victor Toso]

Hi,

On Thu, May 25, 2017 at 10:43:45AM -0500, Jonathon Jongsma wrote:
> On Wed, 2017-05-24 at 14:32 +0200, Victor Toso wrote:
> > From: Victor Toso <me at victortoso.com>
> >
> > Most of spice-gtk code tries to follow that. Doing this specifically
> > to the clipboard_get_targets() due to the recent previous changes in
> > this function.
> >
> > This patch also takes the opportunity to remove the array
> > initialization of types[] which is not needed since 1b73ae3cf25bd4
> > "gtk-session: use clear variable for array's size"
> >
> > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > ---
> >  src/spice-gtk-session.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > index 33db3c8..337ba78 100644
> > --- a/src/spice-gtk-session.c
> > +++ b/src/spice-gtk-session.c
> > @@ -603,6 +603,11 @@ static void clipboard_get_targets(GtkClipboard
> > *clipboard,
> >                                    gpointer user_data)
> >  {
> >      SpiceGtkSession *self = free_weak_ref(user_data);
> > +    SpiceGtkSessionPrivate *s;
> > +    guint32 types[SPICE_N_ELEMENTS(atom2agent)];
> > +    gint num_types;
> > +    int a;
> > +    int selection;
> >  
> >      SPICE_DEBUG("%s:", __FUNCTION__);
> >  
> > @@ -616,12 +621,7 @@ static void clipboard_get_targets(GtkClipboard
> > *clipboard,
> >          return;
> >      }
> >  
> > -    SpiceGtkSessionPrivate *s = self->priv;
> > -    guint32 types[SPICE_N_ELEMENTS(atom2agent)] = { 0 };
> > -    gint num_types;
> > -    int a;
> > -    int selection;
> > -
> > +    s = self->priv;
> >      if (s->main == NULL)
> >          return;
> >  
>
> Personally, I disagree with removing the array initialization. In
> general I think it's safer to initialize variables immediately when
> they're declared, even if it's not strictly necessary. In fact, I'd
> recommend doing this *more* often rather than less often (for example,
> initialize num_types to 0 in the declaration as well rather than
> waiting until later). This reduces the chance that you'll accidentally
> forget to initialize a variable and end up using random memory
> (potentially resulting in memory corruption).
>
> Jonathon

To be honest, I don't mind it that much. Patch has only cosmetic
changes.

I don't disagree with you here about having the array initialization
being safer. As we define variables in the beginning of the function but
I do find useful to let them uninitialized in order to get
-Wuninitialized or -Wmaybe-uninitialized when I'm changing some code
instead of using a null pointer for instance.

I don't disagree with Frediano either, that initializing this is not
necessary as num_types takes care of the size of the array and which
values should be accessed.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170529/afe81d30/attachment.sig>