[PATCH:twm] Add some const.

[Thomas Klausner]

Ah, good catch. I've attached a cleanup patch for that.
 Thomas


On Wed, Sep 23, 2015 at 11:38:02AM -0700, Jasper St. Pierre wrote:
> I was imagining that it might be used like:
> 
>     char *foo = ExpandFilename(name);
>     ...
>     if (foo != name)
>         free(foo);
> 
> ... which would still work, but the != is now dead code. If callers
> were unconditionally freeing before, I heavily suspect something more
> serious here.
> 
> 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
> 
-------------- next part --------------
>From e8cd9fe47d1458d6a5ea17addb5cef7026742c82 Mon Sep 17 00:00:00 2001
From: Thomas Klausner <wiz at NetBSD.org>
Date: Wed, 23 Sep 2015 20:53:24 +0200
Subject: [PATCH:twm 2/2] Adapt callers to ExpandFilename change.

It now always allocates memory, so remove some unnecessary checks.
While here, improve handling of an error case.
---
 src/menus.c | 32 ++++++++++++++++++--------------
 src/util.c  |  4 ++--
 2 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/src/menus.c b/src/menus.c
index e23b5ff..ada9c41 100644
--- a/src/menus.c
+++ b/src/menus.c
@@ -2020,7 +2020,7 @@ ExecuteFunction(int func, const char *action, Window w, TwmWindow *tmp_win,
 				 "%s:  unable to open cut file \"%s\"\n",
 				 ProgramName, tmp);
 		    }
-		    if (ptr != tmp) free (ptr);
+		    free (ptr);
 		}
 	    } else {
 		XFree(ptr);
@@ -2171,21 +2171,25 @@ ExecuteFunction(int func, const char *action, Window w, TwmWindow *tmp_win,
 
     case F_FILE:
 	ptr = ExpandFilename(action);
-	fd = open(ptr, O_RDONLY);
-	if (fd >= 0)
-	{
-	    count = read(fd, buff, MAX_FILE_SIZE - 1);
-	    if (count > 0)
-		XStoreBytes(dpy, buff, count);
+	if (ptr) {
+	    fd = open(ptr, O_RDONLY);
+	    if (fd >= 0)
+	    {
+		count = read(fd, buff, MAX_FILE_SIZE - 1);
+		if (count > 0)
+		    XStoreBytes(dpy, buff, count);
 
-	    close(fd);
-	}
-	else
-	{
-	    fprintf (stderr, "%s:  unable to open file \"%s\"\n",
-		     ProgramName, ptr);
+		close(fd);
+	    }
+	    else
+	    {
+		fprintf (stderr, "%s:  unable to open file \"%s\"\n",
+			 ProgramName, ptr);
+	    }
+	    free(ptr);
+	} else {
+	    fprintf (stderr, "%s: error expanding filename\n", ProgramName);
 	}
-	if (ptr != action) free(ptr);
 	break;
 
     case F_REFRESH:
diff --git a/src/util.c b/src/util.c
index 5e8f0db..8e9dab9 100644
--- a/src/util.c
+++ b/src/util.c
@@ -350,7 +350,7 @@ FindBitmap (const char *name, unsigned *widthp, unsigned *heightp)
     pm = XmuLocateBitmapFile (ScreenOfDisplay(dpy, Scr->screen), bigname, NULL,
 			      0, (int *)widthp, (int *)heightp, &HotX, &HotY);
     if (pm == None && Scr->IconDirectory && bigname[0] != '/') {
-	if (bigname != name) free (bigname);
+	free (bigname);
 	/*
 	 * Attempt to find icon in old IconDirectory (now obsolete)
 	 */
@@ -367,7 +367,7 @@ FindBitmap (const char *name, unsigned *widthp, unsigned *heightp)
 	    pm = None;
 	}
     }
-    if (bigname != name) free (bigname);
+    free (bigname);
     if (pm == None) {
 	fprintf (stderr, "%s:  unable to find bitmap \"%s\"\n",
 		 ProgramName, name);
-- 
2.5.2