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@xxxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > ---
> > 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.
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel