Re: vasnprintf SEGV when %s arg is NULL

[Derek Atkins <warlord MIT EDU>]

David Jafferian <David Jafferian Sun COM> writes:

> Hi -
>
> After about two weeks of evenings, I finally got gnucash 2.0.1 built
> and running on my Solaris 9 x86 PC.  But upon attempting to bring
> up the Edit->Preferences dialog, it received a SIGSEGV :
>
[snip]
>
> I believe Owen's blunt response to be somewhat justified, since it
> would appear that the behavior of functions in the printf family is
> specified to be undefined when the argument to a %s specifier is
> NULL.  Some implementations will substitute a string such as
> "(nil)" as an indicator, but the gnulib/glib implementation clearly
> expects a valid pointer.

Yeah, within the gnucash code we've tried to fix this.  In particular
we tend to use a construct like:

   (foo ? foo : "(null)")

in places where a string argument to printf can be NULL.  You'll find
this construct used throughout the gnucash code.

> Rooting out the problem :
>
>> d22456cb/s
> 0xd22456cb:     %s-client
>
>>From "gnucash-2.0.1/src/core-utils/gnc-gconf-utils.c" :
> void
> gnc_gconf_add_notification (GObject *object,
> 			    const gchar *section,
> 			    GConfClientNotifyFunc callback)
> ...
> 	client_tag = g_strdup_printf(CLIENT_TAG, section);
>
> "section" should not be NULL.

Well, if it should not be NULL, perhaps there should be a g_assert()
in there?  How do you know that "section" isn't supposed to be NULL?
Or are you inferring this from the Solaris printf() problem?

>>From "gnucash-2.0.1/src/gnome-utils/dialog-preferences.c" :
> void
> gnc_preferences_dialog (void)
> ...
>   gnc_gconf_add_notification(G_OBJECT(dialog), NULL,
> 			     gnc_preferences_gconf_changed);
>
> It looks like that NULL has been there since this function was first
> created, but something should replace it.  I don't know enough
> about this code to suggest what should be put there, but replacing
> NULL with "(nil)" made the crash go away.

Yep.  See above..

Maybe David will respond to this.

>               Only problem left was
> another SEGV crash upon closing the preferences dialog :
>
[snip]
>
> And here is the cause :
>
>>From "gnucash-2.0.1/src/gnome-utils/dialog-preferences.c" :
> void
> gnc_preferences_response_cb(GtkDialog *dialog, gint response, GtkDialog *unused)
> ...
>      gnc_gconf_remove_notification(G_OBJECT(dialog), NULL);

Looks like a similar issue.

> If this were the extent of the problem the solution would be simple.
> But then I hit the same problem while running gedit :
>
[snip]
>
> Here the NULL is not hardcoded, so the fix would be a bit more
> complex, but the real issue here is the suggestion that there may
> be hundreds of these little bugs sprinkled throughout all of those
> libraries and applications which depend on glib.

Um... This isn't gnucash code, so...  This part doesn't belong on
this list.

> Although Owen may have been correct, it would not be grossly
> incorrect to deal with this issue by reimplementing vasnprintf()
> in glib to substitute a constant indicator string, e.g. "(nil)", for any
> NULL argument to a %s specifier.
>
> What has been the general consensus on this ?

Well, within gnucash we've tried to not pass NULL down into those
functions.  You'd have to talk to the glib developers to ask about
"fixing" the underlying functions.

> David N. Jafferian
> Systems Technology Service Center
> Sun Microsystems, Inc.

-derek

-- 
       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
       Member, MIT Student Information Processing Board  (SIPB)
       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
       warlord MIT EDU                        PGP key available