GNOME Bugzilla – Bug 710787
Replace most uses of sprintf() with g_snprintf()
Last modified: 2013-10-26 00:41:32 UTC
Created attachment 257998 [details] [review] 0001-Replace-most-uses-of-sprintf-with-g_snprintf.patch sprintf() can potentially overwrite the buffer, but snprintf() takes a length to stop it from doing that. Some of these might instead be replaced by g_strdup_printf(). It is hard to know if the use of alloca() and sprintf() was just because g_strdup_printf() did not exist yet, or if alloca() was used for performance.
Created attachment 258000 [details] [review] 0002-db-scrap-tools-Free-some-leaked-strings.patch This one also fixes some leaks, though this file doesn't seem to be used. Note that I have not tested either of these patches for anything other than compilation.
I'd also like to produce a similar patch replacing use of strcpy() with g_strlcpy() or g_strdup().
Thanks for the patches. Indeed these unsafe functions need to go. (In reply to comment #0) > Some of these might instead be replaced by g_strdup_printf(). It is hard to > know if the use of alloca() and sprintf() was just because g_strdup_printf() > did not exist yet, or if alloca() was used for performance. Especially in Camel, which does lots of disk and network I/O throughout, that kind of micro-optimization doesn't make a bit of difference. Personally I only ever use alloca() if I'm extending a string I know to be relatively short by some fixed number of bytes. Otherwise I prefer g_strdup_printf(). My only nit-pick would be to just use sizeof() on gchar buffers instead of G_N_ELEMENTS(), since the macro just expands to sizeof() / 1. But I can change that when I commit it.
Committed the first patch with some tweaks for E-D-S 3.11.2 and 3.10.2: https://git.gnome.org/browse/evolution-data-server/commit/?id=ad79ca023cb34175920a0c0fbebf3218f2f4e9eb https://git.gnome.org/browse/evolution-data-server/commit/?h=gnome-3-10&id=23f304947463c972e60a94fa3cad91812f837f1c As for db-scrap-tools, I have no idea what that was for so I deleted it.