On Sep 23, 11:38am, [email protected] ("Jasper St. Pierre") wrote: -- Subject: Re: [PATCH:twm] Add some const.
| I was imagining that it might be used like: | | char *foo = ExpandFilename(name); | ... | if (foo != name) | free(foo); Yes, that's how it is currently done. | ... which would still work, but the != is now dead code. If callers | were unconditionally freeing before, I heavily suspect something more | serious here. I am trying to fix: http://cgit.freedesktop.org/xorg/app/twm/tree/src/menus.c#n1291 "action" is "const char *" http://cgit.freedesktop.org/xorg/app/twm/tree/src/menus.c#n2173 ExpandFilename(action); I wanted the minimal diffs; the code could use a lot more cleanup. Please feel free to make it better! christos | | On Wed, Sep 23, 2015 at 9:30 AM, Thomas Klausner <[email protected]> wrote: | > Well, yes, but for context, here is the full function after the change: | > | > char * | > ExpandFilename(const char *name) | > { | > char *newname; | > | > if (name[0] != '~') return strdup(name); | > | > newname = malloc (HomeLen + strlen(name) + 2); | > if (!newname) { | > fprintf (stderr, | > "%s: unable to allocate %ld bytes to expand filename %s/%s\n", | > ProgramName, HomeLen + (unsigned long)strlen(name) + 2, | > Home, &name[1]); | > } else { | > (void) sprintf (newname, "%s/%s", Home, &name[1]); | > } | > | > return newname; | > } | > | > So in other words, now the function is consistent in returning a | > malloc()ed string when it succeeds. | > Thomas | > | > On Wed, Sep 23, 2015 at 08:59:22AM -0700, Jasper St. Pierre wrote: | >> Should also mention that it also adds a strdup -- I feel that the code | >> calling this might conditionally free. Would be nice to double-check | >> callers. | >> | >> On Wed, Sep 23, 2015 at 1:58 AM, Thomas Klausner <[email protected]> wrote: | >> > From: Christos Zoulas <[email protected]> | >> > | >> > Signed-off-by: Thomas Klausner <[email protected]> | >> > --- | >> > src/util.c | 4 ++-- | >> > src/util.h | 2 +- | >> > 2 files changed, 3 insertions(+), 3 deletions(-) | >> > | >> > diff --git a/src/util.c b/src/util.c | >> > index 4b94051..5e8f0db 100644 | >> > --- a/src/util.c | >> > +++ b/src/util.c | >> > @@ -256,11 +256,11 @@ Zoom(Window wf, Window wt) | >> > * \param name the filename to expand | >> > */ | >> > char * | >> > -ExpandFilename(char *name) | >> > +ExpandFilename(const char *name) | >> > { | >> > char *newname; | >> > | >> > - if (name[0] != '~') return name; | >> > + if (name[0] != '~') return strdup(name); | >> > | >> > newname = malloc (HomeLen + strlen(name) + 2); | >> > if (!newname) { | >> > diff --git a/src/util.h b/src/util.h | >> > index 7f4527c..4b2d3a8 100644 | >> > --- a/src/util.h | >> > +++ b/src/util.h | >> > @@ -64,7 +64,7 @@ in this Software without prior written authorization from The Open Group. | >> > extern void MoveOutline ( Window root, int x, int y, int width, int height, | >> > int bw, int th ); | >> > extern void Zoom ( Window wf, Window wt ); | >> > -extern char * ExpandFilename ( char *name ); | >> > +extern char * ExpandFilename ( const char *name ); | >> > extern void GetUnknownIcon ( const char *name ); | >> > extern Pixmap FindBitmap ( const char *name, unsigned int *widthp, | >> > unsigned int *heightp ); | >> > -- | >> > 2.5.2 | >> > | >> > _______________________________________________ | >> > [email protected]: X.Org development | >> > Archives: http://lists.x.org/archives/xorg-devel | >> > Info: http://lists.x.org/mailman/listinfo/xorg-devel | >> | >> | >> | >> -- | >> Jasper | >> | | | | -- | Jasper -- End of excerpt from "Jasper St. Pierre" _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
