On Wed, 2005-03-30 at 19:42, John Gotts wrote:
> In message <1112223809.3671.7.camel@xxxxxxxxxxxxxxxxxxxxx>, Ray Strode writes:
>
> >> We've had the same code in our software since 2000:
>
> >> GnomeFileEntry *fe;
> >> gchar buf[1024];
>
> >> ---> sprintf(buf, "%s", gnome_file_entry_get_full_path(fe, TRUE));
>
> >...
> >> if (strcmp(buf, "(null)") == 0)
> >> *buf = '\0';
>
> >get_full_path allocates memory for you. sprintf is a bad idea because
> >there is no bounds checking, you're leaking the memory that
> >get_full_path allocates, and you're not supposed to pass NULL to sprintf
> >when it wants a character array. doing if (strcmp(buf, "(null)") is
> >bad, too.
>
> >instead do:
> >gchar *buf;
> >...
> >buf = gnome_file_entry_get_full_path (fe, TRUE));
>
> >Then you can check for NULL;
>
> >if (buf == NULL)
> > buf = g_strdup ("");
>
> >and then call g_free when you're done with it.
>
> >> Fortunately, the file entry widget was for
> >> future use, so I completely removed its use in our code.
> >That's an even better idea.
>
> It's an incredibly wasteful exercise to criticize three lines of code that was
> intended to demonstrate that a stable (5+ year old) library was broken.
I don't think Ray was criticizing the three lines of code but just
pointing out the misuse of the API. API's are written with certain
assumptions. If you deviate from those assumptions the code may break
at some later date no matter how long it had been working in the past
(i.e. the code may have been working because of a bug in the API that
was later fixed).
BTW your stack trace is no good. There is no context where the strcpy
is happening. Are you sure it isn't crashing in the sprintf? Try
separating the code and perhaps it will make it easier to debug or at
least break and try to step into that code.
--
J5
