Re: bounded string ops in wings
On Mon, 20 Sep 2010 at 22:43:58 +0200, Tamas TEVESZ wrote:
so far only wings is done (mostly), no ill effects seen in a couple of
hours of use.
Just a couple of trivial things, while trying to digest such a delicate
patch.
--- a/WINGs/findfile.c
+++ b/WINGs/findfile.c
@@ -84,9 +84,8 @@ char *wexpandpath(char *path)
path++;
if (*path == '/' || *path == 0) {
home = wgethomedir();
- if (strlen(home) PATH_MAX)
+ if (strlcpy(buffer, home, sizeof(buffer)) =
sizeof(buffer))
goto error;
- strcat(buffer, home);
} else {
int j;
j = 0;
@@ -98,9 +97,8 @@ char *wexpandpath(char *path)
path++;
}
home = getuserhomedir(buffer2);
- if (!home || strlen(home) PATH_MAX)
+ if (!home || strlcat(buffer, home, sizeof(buffer)) =
sizeof(buffer))
goto error;
- strcat(buffer, home);
I was wondering why you replaced the first strcat() with strlcpy() while
the second with strlcat().
/* return address of next char != tok or end of string whichever comes first
*/
@@ -251,9 +253,16 @@ char *wfindfile(char *paths, char *file)
path = wmalloc(len + flen + 2);
path = memcpy(path, tmp, len);
path[len] = 0;
- if (path[len - 1] != '/')
- strcat(path, /);
- strcat(path, file);
+ if (path[len - 1] != '/') {
+ if (strlcat(path, /, len + flen + 2) = len + flen +
2) {
+ wfree(path);
+ return NULL;
+ }
+ }
+ if (strlcat(path, file, len + flen + 2) = len + flen + 2) {
+ wfree(path);
+ return NULL;
+ }
fullpath = wexpandpath(path);
wfree(path);
if (fullpath) {
@@ -301,8 +310,11 @@ char *wfindfileinlist(char **path_list, char *file)
path = wmalloc(len + flen + 2);
path = memcpy(path, path_list[i], len);
path[len] = 0;
- strcat(path, /);
- strcat(path, file);
+ if (strlcat(path, /, len + flen + 2) = len + flen + 2 ||
+ strlcat(path, file, len + flen + 2) = len + flen + 2) {
+ wfree(path);
+ return NULL;
+ }
/* expand tilde */
fullpath = wexpandpath(path);
wfree(path);
@@ -358,8 +370,11 @@ char *wfindfileinarray(WMPropList * array, char *file)
path = wmalloc(len + flen + 2);
path = memcpy(path, p, len);
path[len] = 0;
- strcat(path, /);
- strcat(path, file);
+ if (strlcat(path, /, len + flen + 2) = len + flen + 2 ||
+ strlcat(path, file, len + flen + 2) = len + flen + 2) {
+ wfree(path);
+ return NULL;
+ }
Please put all these multiple wfree(path); return NULL; going into one
'goto error', here and in other places in the patch too. You will also
save the { } for the if's, as they become one-liners.
+static void normalizePath(char *s)
+{
+ int i, j, found;
+
+ found = 0;
+ for(i = 0; s[i]; !found i++) {
'for' is not a function, so for (...)
+ found = 0;
+ if (s[i] == '/' s[i+1] == '/') {
+ int nslash = 1;
+ found = 1;
+ i++;
+ while(s[i+nslash] == '/')
the same for 'while'
+ nslash++;
+ for(j = 0; s[i+j+nslash]; j++)
again
+ s[i+j] = s[i+j+nslash];
+ s[i+j] = '\0';
+ }
+ }
+ if (i 1 s[--i] == '/')
+ s[i] = '\0';
+}
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: two quickies
On Tue, 21 Sep 2010 at 3:03:40 +0200, Tamas TEVESZ wrote:
how about this:
diff --git a/WINGs/memory.c b/WINGs/memory.c
index 9d770b0..bb6df5c 100644
--- a/WINGs/memory.c
+++ b/WINGs/memory.c
@@ -95,6 +95,7 @@ void *wmalloc(size_t size)
}
}
}
+ memset(tmp, 0, size);
return tmp;
}
Hm, I'd rather keep them separate and fix the omissions.
on a different note, i'm quite sure about this one:
diff --git a/src/application.c b/src/application.c
index 21155c7..ca1cf1a 100644
--- a/src/application.c
+++ b/src/application.c
@@ -192,7 +192,7 @@ void wApplicationExtractDirPackIcon(WScreen * scr, char
*path, char *wm_instance
if (access(tmp, R_OK) == 0)
iconPath = tmp;
}
- if (!path) {
+ if (!iconPath) {
strcpy(tmp, path);
strcat(tmp, .xpm);
if (access(tmp, R_OK) == 0)
Yep! Mind doing a separate patch for this one?
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: bounded string ops in wings
On Tue, 21 Sep 2010, Carlos R. Mafra wrote:
--- a/WINGs/findfile.c
+++ b/WINGs/findfile.c
@@ -84,9 +84,8 @@ char *wexpandpath(char *path)
path++;
if (*path == '/' || *path == 0) {
home = wgethomedir();
- if (strlen(home) PATH_MAX)
+ if (strlcpy(buffer, home, sizeof(buffer)) =
sizeof(buffer))
goto error;
- strcat(buffer, home);
} else {
int j;
j = 0;
@@ -98,9 +97,8 @@ char *wexpandpath(char *path)
path++;
}
home = getuserhomedir(buffer2);
- if (!home || strlen(home) PATH_MAX)
+ if (!home || strlcat(buffer, home, sizeof(buffer)) =
sizeof(buffer))
goto error;
- strcat(buffer, home);
I was wondering why you replaced the first strcat() with strlcpy() while
the second with strlcat().
it doesn't really matter. if you are just starting to fill up a
storage, cat and cpy do the same thing. for consistency's sake i'll
make them consistent, thanks for pointing it out.
@@ -358,8 +370,11 @@ char *wfindfileinarray(WMPropList * array, char *file)
path = wmalloc(len + flen + 2);
path = memcpy(path, p, len);
path[len] = 0;
- strcat(path, /);
- strcat(path, file);
+ if (strlcat(path, /, len + flen + 2) = len + flen + 2 ||
+ strlcat(path, file, len + flen + 2) = len + flen + 2) {
+ wfree(path);
+ return NULL;
+ }
Please put all these multiple wfree(path); return NULL; going into one
'goto error', here and in other places in the patch too. You will also
save the { } for the if's, as they become one-liners.
thank you :) (lively demonstration of not all gotos are bad)
+ for(i = 0; s[i]; !found i++) {
'for' is not a function, so for (...)
ok.
--
[-]
mkdir /nonexistent
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Fix typo in wApplicationExtractDirPackIcon()
From 9aaeac4d74b517e8d3ff8a8759c3ff7da258a21b Mon Sep 17 00:00:00 2001
From: Tamas TEVESZ i...@extreme.hu
Date: Tue, 21 Sep 2010 12:30:02 +0200
Subject: [PATCH] Fix typo in wApplicationExtractDirPackIcon()
Signed-off-by: Tamas TEVESZ i...@extreme.hu
---
src/application.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/application.c b/src/application.c
index c5f2c56..7913792 100644
--- a/src/application.c
+++ b/src/application.c
@@ -192,7 +192,7 @@ void wApplicationExtractDirPackIcon(WScreen * scr, char
*path, char *wm_instance
if (access(tmp, R_OK) == 0)
iconPath = tmp;
}
- if (!path) {
+ if (!iconPath) {
strcpy(tmp, path);
strcat(tmp, .xpm);
if (access(tmp, R_OK) == 0)
--
1.7.0.4
--
[-]
mkdir /nonexistent
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
autoconf wizardry needed
hi, i found out that that (at least) several linuxes ship libbsd, which (among other things) have strlcat/strlcpy. it would be preferable to pick these up instead of the bundled ones. i need someone to turn AC_CHECK_FUNC(strlcat, AC_DEFINE(HAVE_STRLCAT, 1, Check for strlcat)) into something along the lines of if (strlcat is in libc) define HAVE_STRLCAT else if (strlcat is in libbsd) define HAVE_STRLCAT define HAVE_LIBBSD add -lbsd to LIBS (wutil libs) else just move along and likewise for strlcpy (or, up to you, we could just assume whereever there's strlcat, there's also strlcpy, and only check for one of them). anyone likes a nice challenge? :) thanks, -- [-] mkdir /nonexistent -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: two quickies
On Tue, 21 Sep 2010, Carlos R. Mafra wrote: diff --git a/WINGs/memory.c b/WINGs/memory.c index 9d770b0..bb6df5c 100644 --- a/WINGs/memory.c +++ b/WINGs/memory.c @@ -95,6 +95,7 @@ void *wmalloc(size_t size) } } } + memset(tmp, 0, size); return tmp; } Hm, I'd rather keep them separate and fix the omissions. but why? there are a hundred-plus places with separate memset(0) calls after w?malloc, and god knows how many where the intent is expressed in variations of *p = '\0' (these forms i'm not really fond of, but i may be overly cautious). -- [-] mkdir /nonexistent -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: autoconf wizardry needed
On Tue, 21 Sep 2010, Andreas Tscharner wrote: if (strlcat is in libc) define HAVE_STRLCAT else if (strlcat is in libbsd) define HAVE_STRLCAT define HAVE_LIBBSD add -lbsd to LIBS (wutil libs) else just move along and likewise for strlcpy (or, up to you, we could just assume whereever there's strlcat, there's also strlcpy, and only check for one of them). anyone likes a nice challenge? :) What about AC_CHECK_LIB? As far as I can see, it looks like you could use it and it does by default what you need. For detailed information see here: http://www.gnu.org/software/hello/manual/autoconf/Libraries.html yep, theoretically i know, practically i couldn't ;) i tried. what i was unable to describe the autoconf way is that in the `strlcat is in libbsd' case there are two actions to take, define both HAVE_STRLCAT and HAVE_LIBBSD (the libs addition could probably be shoved in later). -- [-] mkdir /nonexistent -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: autoconf wizardry needed
On 21.09.2010 13:49, Tamas TEVESZ wrote: On Tue, 21 Sep 2010, Andreas Tscharner wrote: if (strlcat is in libc) define HAVE_STRLCAT else if (strlcat is in libbsd) define HAVE_STRLCAT define HAVE_LIBBSD add -lbsd to LIBS (wutil libs) else just move along and likewise for strlcpy (or, up to you, we could just assume whereever there's strlcat, there's also strlcpy, and only check for one of them). anyone likes a nice challenge? :) What about AC_CHECK_LIB? As far as I can see, it looks like you could use it and it does by default what you need. For detailed information see here: http://www.gnu.org/software/hello/manual/autoconf/Libraries.html yep, theoretically i know, practically i couldn't ;) i tried. what i was unable to describe the autoconf way is that in the `strlcat is in libbsd' case there are two actions to take, define both HAVE_STRLCAT and HAVE_LIBBSD (the libs addition could probably be shoved in later). AC_CHECK_LIB(c, strlcat, [STRLCAT_IN_LIBC=yes], [STRLCAT_IN_LIBC=no]) AC_CHECK_LIB(bsd, strlcat, [STRLCAT_IN_LIBBSD=yes], [STRLCAT_IN_LIBBSD=no]) ... ... AM_CONDITIONAL(HAVE_STRLCAT, test $STRLCAT_IN_LIBC = yes) AM_CONDITIONAL(HAVE_STRLCAT, test $STRLCAT_IN_LIBBSD = yes) AM_CONDITIONAL(HAVE_LIBBSD, test $STRLCAT_IN_LIBBSD = yes) Not very elegant and I could not test it... Best regards Andreas -- (`-''-/).___..--''`-._ `o_ o ) `-. ( ).`-.__.`) (_Y_.)' ._ ) `._ `. ``-..-' _..`--'_..-_/ /--'_.' .' (il).-'' (li).' ((!.-' Andreas Tscharner a...@vis.ethz.ch ICQ-No. 14356454 -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
who's responsible for ./debian directory ?
People, i don't know for sure who mantains the content of the debian directory, with all the stuff needed to generate .deb packages. are the guys on debian's many lists or the nice people from this one ? i'm asking this because i've been working on a new menu-method script that debian uses to create menus for many diferent kinds of window managers. what my script does different than the current method on debian, is generate a proplist, instead of an old menu.hook. right now, it does most of the things right, but it still need some testing to see if there's any bug left. if i send my menu-method to this list, and it's sound (i.e. i squash the few remaining bugs), will it be picked by debian when they pull wmaker-crm ? or should i send it to debian too ? Bento Loewenstein -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: autoconf wizardry needed
On 21.09.2010 12:51, Tamas TEVESZ wrote: i need someone to turn AC_CHECK_FUNC(strlcat, AC_DEFINE(HAVE_STRLCAT, 1, Check for strlcat)) into something along the lines of if (strlcat is in libc) define HAVE_STRLCAT else if (strlcat is in libbsd) define HAVE_STRLCAT define HAVE_LIBBSD add -lbsd to LIBS (wutil libs) else just move along Is there really a need to define HAVE_LIBBSD? All we care about is HAVE_STRLCAT, no matter how it is available. (or, up to you, we could just assume whereever there's strlcat, there's also strlcpy, and only check for one of them). That's probably a warranted assumption. On Tue, Sep 21, 2010 at 01:06:25PM +0200, Andreas Tscharner wrote: What about AC_CHECK_LIB? AC_SEARCH_LIBS is more like it, it really does do exactly what we want here. dnl Check for strlcat dnl = AC_SEARCH_LIBS([strlcat],[bsd],[AC_DEFINE(HAVE_STRLCAT, 1, [Define if strlcat is available])]) Works correctly for me, at any rate. -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: who's responsible for ./debian directory ?
On Tue, Sep 21, 2010 at 10:57:56AM -0300, Bento Loewenstein wrote: what my script does different than the current method on debian, is generate a proplist, instead of an old menu.hook. right now, it does most of the things right, but it still need some testing to see if there's any bug left. This was discussed a few months ago too, see the thread starting at http://lists.windowmaker.info/dev/msg01337.html Are you managing to keep the appearance.menu stuff that uses OPEN_MENU and cpp? -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: [PATCH] Add a preference for XUrgencyHint bouncing
Quoth Brad Jorsch, Signed-off-by: Brad Jorschano...@users.sourceforge.net --- WPrefs.app/Expert.c |8 +++- src/WindowMaker.h |3 +++ src/defaults.c |2 ++ src/superfluous.c |2 +- 4 files changed, 13 insertions(+), 2 deletions(-) Great. At some point I may send a patch to completely disable bouncing but I imagine most people will be happy with being able to stop icons from demanding attention and I don't have time to do anything right now. I have actually tried out the bouncing at last. It certainly is nicely done if you like that sort of thing. -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: autoconf wizardry needed
On Tue, 21 Sep 2010, Brad Jorsch wrote: i need someone to turn AC_CHECK_FUNC(strlcat, AC_DEFINE(HAVE_STRLCAT, 1, Check for strlcat)) into something along the lines of if (strlcat is in libc) define HAVE_STRLCAT else if (strlcat is in libbsd) define HAVE_STRLCAT define HAVE_LIBBSD add -lbsd to LIBS (wutil libs) else just move along Is there really a need to define HAVE_LIBBSD? All we care about is HAVE_STRLCAT, no matter how it is available. there could be a need, but i'm not even close to knowing squat about autoconf, so i may be really wrong. on the bsds (where these functions are in libc), they can be had by simply including string.h. on linux with libbsd, bsd/string.h is needed too (in addition to the normal string.h), so some sort of marker is needed to decide whether or not to include bsd/string.h. am i wrong thinking this? also, in the libbsd case, -lbsd needs to be added to the wutil/wings libs (XXX clean up where and how), whereas on a real bsd, no additional nothing is needed, it just comes with libc. dnl Check for strlcat dnl = AC_SEARCH_LIBS([strlcat],[bsd],[AC_DEFINE(HAVE_STRLCAT, 1, [Define if strlcat is available])]) Works correctly for me, at any rate. i'll give that a try, thanks. -- [-] mkdir /nonexistent -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: Configure parameter missing
On Tue, 21 Sep 2010 19:00:21 +0200 (CEST) Tamas TEVESZ i...@wormhole.hu wrote: On Tue, 21 Sep 2010, Daniel Isenmann wrote: hi, tried to compile latest wmaker-crm git fork (master tree). configure doesn't know parameter --with-gnustepdir, but it's listed in configure help. Is this option still available or was it removed from configure script? If it was removed, which is the default GNUStep directory (is it /usr/lib/GNUstep)? apparently it's an error in the message. it's --appspath now (don't know what it does, but configure.ac quacks appspath...) No, that's not right. Giving --appspath to configure fails completely. --with-gnustepdir works, but configure gives the error message, that it isn't recognized, but it sets the right path. You can see it after configure: without gnustepdir: Installation path for WPrefs.app: /usr with gnustepdir set to /usr/lib/GNUstep/: Installation path for WPrefs.app: /usr/lib/GNUstep/Applications So, there is some error in configure script or configure.ac -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: autoconf wizardry needed
On Tue, Sep 21, 2010 at 06:32:20PM +0200, Tamas TEVESZ wrote: On Tue, 21 Sep 2010, Brad Jorsch wrote: Is there really a need to define HAVE_LIBBSD? All we care about is HAVE_STRLCAT, no matter how it is available. on the bsds (where these functions are in libc), they can be had by simply including string.h. on linux with libbsd, bsd/string.h is needed too (in addition to the normal string.h), so some sort of marker is needed to decide whether or not to include bsd/string.h. am i wrong thinking this? Maybe. What you'll probably end up doing is providing fallback implementations if HAVE_STRLCAT is not defined, so no matter what the functions will exist. And in that case, the prototypes will have to be provided in some local .h file. Rather than making that .h file conditional on HAVE_STRLCAT as well, you might just provide the prototypes unconditionally. OTOH, if it is somehow likely that the bsd or libbsd headers will use different types (e.g. int instead of size_t, or something like that) you wouldn't want to do that. also, in the libbsd case, -lbsd needs to be added to the wutil/wings libs (XXX clean up where and how), whereas on a real bsd, no additional nothing is needed, it just comes with libc. That is automatically taken care of by AC_SEARCH_LIBS. BTW, you may want to provide a configure option to skip the check for libbsd, in case a distro wants to make sure they don't pull in a dependency on libbsd. Something like this: AC_ARG_WITH([libbsd], [AS_HELP_STRING([--without-libbsd], [do not use libbsd for strlcat and strlcpy [default=check]])], [], [with_libbsd=check]) AS_IF([test x$with_libbsd != xno], [AC_SEARCH_LIBS([strlcat],[bsd], [AC_DEFINE(HAVE_STRLCAT, 1, [Define if strlcat is available])], [if test x$with_libbsd != xcheck; then AC_MSG_FAILURE([--with-libbsd was given, but test for libbsd failed]) fi ] )] ) -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: autoconf wizardry needed
On Tue, 21 Sep 2010, Brad Jorsch wrote: Maybe. What you'll probably end up doing is providing fallback implementations if HAVE_STRLCAT is not defined, so no matter what the functions will exist. And in that case, the prototypes will have to be provided in some local .h file. yes, this part is already taken care of. BTW, you may want to provide a configure option to skip the check for libbsd, in case a distro wants to make sure they don't pull in a dependency on libbsd. Something like this: so far my best effort is AC_ARG_WITH([libbsd], [AS_HELP_STRING([--without-libbsd], [do not use libbsd for strlcat and strlcpy [default=check]])], [], [with_libbsd=check]) AS_IF([test x$with_libbsd != xno], [AC_SEARCH_LIBS([strlcat],[bsd], [AC_DEFINE(HAVE_STRLCAT, 1, [Define if strlcat is available])], [if test x$with_libbsd != xcheck; then AC_MSG_FAILURE([--with-libbsd was given, but test for libbsd failed]) fi ] )] ) case $LIBS in *-lbsd*) AC_DEFINE(HAVE_LIBBSD, 1, [Define if libbsd is used]) ;; esac and providing a custom wstring.h, which includes string.h, and bsd/string.h if needed. it's quite ugly, and this unconditionally links libbsd to every binary, even those not needing it... or, on second thought, i could just let this be, and if it annoys someone (probably debian) too much, they'll fix it : -- [-] mkdir /nonexistent -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: autoconf wizardry needed
On Tue, 21 Sep 2010, Brad Jorsch wrote:
and this unconditionally links libbsd to every binary, even those not
needing it...
One easy fix for that is to grab ax_check_linker_flags.m4[1] and add
this rule to configure.ac:
AX_CHECK_LINKER_FLAGS([-Wl,--as-needed],
[LDFLAGS=$LDFLAGS -Wl,--as-needed],
[:]
)
this would be useful in it's own right, wouldn't it? it seems to do
good to some of utils/.
while here.. is there any point in WM_CHECK_{LIB,HEADER} (from
m4/windowmaker.m4)? what do they do AC_CHECK_{LIB,HEADER} doesn't?
just asking because if we also declare that xft version checking is
not needed anymore (2.1.0 is at least 7 years old), m4/windowmaker.m4
could also be retired.
[1] http://www.gnu.org/software/autoconf-archive/ax_check_linker_flags.html
--
[-]
mkdir /nonexistent
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
