[PATCH:twm] Add some const.

[Christos Zoulas]

On Sep 23, 11:38am, jstpierre at mecheye.net ("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 <wiz at netbsd.org> 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 <wiz at netbsd.org> wrote:
| >> > From: Christos Zoulas <christos at NetBSD.org>
| >> >
| >> > Signed-off-by: Thomas Klausner <wiz at NetBSD.org>
| >> > ---
| >> >  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
| >> >
| >> > _______________________________________________
| >> > xorg-devel at lists.x.org: 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"