On Wed, Jul 29, 2015 at 05:08:18PM +0300, Pavel Fedin wrote:
> Hello!
>
> > > + l = strlen(name);
> > > + propname = g_malloc(l + 13); /* 10 characters for UINT_MAX plus "[]"
> > > */
> > > + memcpy(propname, name, l);
> >
> > Please don't do manual string length calculations in combination with
> > unbounded sprintf calls. It is a recipe for future security bugs.
>
> [skip]
>
> > > for (i = gpio_list->num_in; i < gpio_list->num_in + n; i++) {
> > > + g_sprintf(&propname[l], "[%u]", i);
> >
> > Replace this with
> >
> > gchar *propname = g_strdup_printf("%s[%u]", name, i)
> >
> > > object_property_add_child(OBJECT(dev), propname,
> > > OBJECT(gpio_list->in[i]),
> > > &error_abort);
> >
> > g_free(propname);
>
> IMHO it's not really good because of repeating allocation-free. This
> is not VERY slow, but still slower than it could be (imagine that this
> repeats ~1000 times).
Unless this repeated allocation overhead is illustrated to be a real world
problem, I think using g_strdup_printf is preferrable.
> I have a better idea instead. What if instead:
>
> propname = g_malloc(l + 13); /* 10 characters for UINT_MAX plus "[]" */
>
> i do:
>
> propname = g_strdup_printf("%s[%u]", name, -1)
>
> ? This will automatically give me a buffer to fit in the largest possible
> integer.
I think it is premature/unneccesary optimization unless there are bench
marks to show this is a real world problem.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
