[PATCH setxkbmap] Eliminate limitations on path length.
Peter Hutterer
peter.hutterer at who-t.net
Thu Feb 24 20:27:43 PST 2011
On Wed, Feb 16, 2011 at 01:18:20AM +0300, Van de Bugger wrote:
> Thanks for the suggestion, I did not know about asprintf. I thought it was my invention. ;-)
>
> Of course, I would like to use asprintf. But rprintf is still required to check the status returned by asprintf.
>
> I am not sure how to define _GNU_SOURCE macro. I done it with #define in the source.
>
> Ok, here is the attempt #2:
>
> From 528c30b04c815bc2b091072fd2b2c32354b6083f Mon Sep 17 00:00:00 2001
> From: Van de Bugger <van.de.bugger at gmail.com>
> Date: Tue, 15 Feb 2011 23:55:28 +0300
> Subject: [PATCH setxkbmap] Eliminate limitations on path length.
>
> ...by using dynamically allocated buffers. No need in PATH_MAX,
> MAXPATHLEN. No more "Path too long" errors.
this commit message has been critisised in this thread, maybe update it to
reflect the comments?
> ---
> configure.ac | 2 +
> setxkbmap.c | 89 ++++++++++++++++++++++++++++++++++++++++-----------------
> 2 files changed, 64 insertions(+), 27 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index bc16554..2c4381f 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -51,6 +51,8 @@ PKG_CHECK_MODULES(SETXKBMAP, xkbfile x11)
> # Allow checking code with lint, sparse, etc.
> XORG_WITH_LINT
>
> +AC_CHECK_FUNCS([vasprintf])
> +
> AC_CONFIG_FILES([
> Makefile
> man/Makefile])
> diff --git a/setxkbmap.c b/setxkbmap.c
> index 3d3b7d8..9996eed 100644
> --- a/setxkbmap.c
> +++ b/setxkbmap.c
> @@ -23,7 +23,9 @@
> THE USE OR PERFORMANCE OF THIS SOFTWARE.
>
> ********************************************************/
> -
> +
> +#define _GNU_SOURCE
> +#include <stdarg.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <locale.h>
> @@ -36,13 +38,7 @@
> #include <X11/extensions/XKBconfig.h>
> #include <X11/extensions/XKBrules.h>
>
> -#ifndef PATH_MAX
> -#ifdef MAXPATHLEN
> -#define PATH_MAX MAXPATHLEN
> -#else
> -#define PATH_MAX 1024
> -#endif
> -#endif
> +#include "config.h"
>
> #ifndef DFLT_XKB_CONFIG_ROOT
> #define DFLT_XKB_CONFIG_ROOT "/usr/share/X11/xkb"
> @@ -170,6 +166,7 @@ static int deviceSpec = XkbUseCoreKbd;
>
> /***====================================================================***/
>
> +char * rprintf( char const * format, ... );
whitespace issue: "char const *format" please
same goes for the other pointer types in this patch.
> Bool addToList(list_t * list, char *newVal);
> void usage(int argc, char **argv);
> void dumpNames(Bool wantRules, Bool wantCNames);
> @@ -189,6 +186,56 @@ void printKeymap(void);
> /***====================================================================***/
>
> /*
> + Like sprintf, but returns freshly allocated string instead writing it to
> + pre-allocated buffer. Do not forget to free returned strung.
> +*/
> +
> +char *
> +rprintf(char const * format, ...)
you could just do a #ifndef HAVE_VASPRINTF and then implement rprintf as
vasprintf instead. this way, the caller just calls vasprintf and error
handling etc is handled in the caller too.
Cheers,
Peter
> +{
> + va_list args;
> + char * buffer = NULL;
> + #ifdef HAVE_VASPRINTF
> + int rc;
> + va_start(args, format);
> + rc = vasprintf(&buffer, format, args);
> + va_end(args);
> + if (rc < 0)
> + {
> + /* It is not clear whether vasprintf sets errno or not. */
> + ERR1("`vasprintf' returned unexpected error (%d).\n", rc);
> + abort();
> + }
> + #else
> + int allocated = 0;
> + int required = 100;
> + do
> + {
> + buffer = realloc(buffer, required + 1);
> + if (buffer == NULL)
> + {
> + ERR("Out of memory.\n");
> + abort();
> + }
> + allocated = required + 1;
> + va_start(args, format);
> + required = vsnprintf(buffer, allocated, format, args);
> + va_end(args);
> + if (required < 0)
> + {
> + /* It is not clear whether vsnprintf sets errno or not. */
> + ERR1("`vsnprintf' returned unexpected error (%d).\n", required);
> + abort();
> + }
> + }
> + while (required >= allocated);
> + #endif
> + return buffer;
> +}
> +
> +/***====================================================================***/
> +
> +/*
> If newVal is NULL or empty string, the list is cleared.
> Otherwise newVal is added to the end of the list (if it is not present in the list yet).
> */
> @@ -623,7 +670,6 @@ FILE *
> findFileInPath(char *name, char *subdir)
> {
> register int i;
> - char buf[PATH_MAX];
> FILE *fp;
>
> if (name[0] == '/')
> @@ -635,16 +681,11 @@ findFileInPath(char *name, char *subdir)
> }
> for (i = 0; (i < inclPath.num); i++)
> {
> - if (snprintf(buf, PATH_MAX, "%s/%s%s", inclPath.item[i], subdir, name) >=
> - PATH_MAX)
> - {
> - VMSG3(0, "Path too long (%s/%s%s). Ignored.\n", inclPath.item[i],
> - subdir, name);
> - continue;
> - }
> - fp = fopen(buf, "r");
> + char * path = rprintf("%s/%s%s", inclPath.item[i], subdir, name);
> + fp = fopen(path, "r");
> if ((verbose > 7) || ((!fp) && (verbose > 5)))
> - MSG2("%s file %s\n", (fp ? "Found" : "Didn't find"), buf);
> + MSG2("%s file %s\n", (fp ? "Found" : "Didn't find"), path);
> + free(path);
> if (fp != NULL)
> return fp;
> }
> @@ -824,7 +865,6 @@ applyRules(void)
> if (settings.model.src || settings.layout.src || settings.variant.src
> || options.item)
> {
> - char buf[PATH_MAX];
> XkbComponentNamesRec rnames;
>
> if (settings.variant.src < settings.layout.src)
> @@ -852,14 +892,9 @@ applyRules(void)
> * we succeed with */
> for (i = 0; (i < inclPath.num) && (!rules); i++)
> {
> - if (snprintf(buf, PATH_MAX, "%s/rules/%s",
> - inclPath.item[i], rfName) >= PATH_MAX)
> - {
> - VMSG2(0, "Path too long (%s/rules/%s). Ignored.\n",
> - inclPath.item[i], rfName);
> - continue;
> - }
> - rules = XkbRF_Load(buf, settings.locale.value, True, True);
> + char * path = rprintf("%s/rules/%s", inclPath.item[i], rfName);
> + rules = XkbRF_Load(path, settings.locale.value, True, True);
> + free(path);
> }
> }
> if (!rules)
> --
> 1.7.4
>
>
>
>
>
>
>
>
>
>
> On Wed, 2011-02-16 at 07:40 +1000, Peter Hutterer wrote:
> > On Wed, Feb 16, 2011 at 12:15:52AM +0300, Van de Bugger wrote:
> > > From 0aaae5b3c0d6183e2791c30155bae40132a0c779 Mon Sep 17 00:00:00 2001
> > > From: Van de Bugger <van.de.bugger at gmail.com>
> > > Date: Tue, 15 Feb 2011 23:55:28 +0300
> > > Subject: [PATCH setxkbmap] Eliminate limitations on path length.
> > >
> > > ...by using dynamically allocated buffers. No need in PATH_MAX,
> > > MAXPATHLEN. No more "Path too long" errors.
> > > ---
> > > setxkbmap.c | 72 ++++++++++++++++++++++++++++++++++++----------------------
> > > 1 files changed, 45 insertions(+), 27 deletions(-)
> > >
> > > diff --git a/setxkbmap.c b/setxkbmap.c
> > > index 3d3b7d8..6777b1b 100644
> > > --- a/setxkbmap.c
> > > +++ b/setxkbmap.c
> > > @@ -24,6 +24,7 @@
> > >
> > > ********************************************************/
> > >
> > > +#include <stdarg.h>
> > > #include <stdio.h>
> > > #include <stdlib.h>
> > > #include <locale.h>
> > > @@ -36,14 +37,6 @@
> > > #include <X11/extensions/XKBconfig.h>
> > > #include <X11/extensions/XKBrules.h>
> > >
> > > -#ifndef PATH_MAX
> > > -#ifdef MAXPATHLEN
> > > -#define PATH_MAX MAXPATHLEN
> > > -#else
> > > -#define PATH_MAX 1024
> > > -#endif
> > > -#endif
> > > -
> > > #ifndef DFLT_XKB_CONFIG_ROOT
> > > #define DFLT_XKB_CONFIG_ROOT "/usr/share/X11/xkb"
> > > #endif
> > > @@ -170,6 +163,7 @@ static int deviceSpec = XkbUseCoreKbd;
> > >
> > > /***====================================================================***/
> > >
> > > +char * rprintf( char const * format, ... );
> > > Bool addToList(list_t * list, char *newVal);
> > > void usage(int argc, char **argv);
> > > void dumpNames(Bool wantRules, Bool wantCNames);
> > > @@ -189,6 +183,42 @@ void printKeymap(void);
> > > /***====================================================================***/
> > >
> > > /*
> > > + Like sprintf, but returns freshly allocated string instead writing it to
> > > + pre-allocated buffer. Do not forget to free returned strung.
> > > +*/
> >
> > this sounds like a job for asprintf if we have it. I think the server has
> > reimplementations for this on non-supporting platforms, maybe we can re-use
> > those.
> >
> > rest of the patch looks good though.
> >
> > Cheers,
> > Peter
> >
>
>
More information about the xorg-devel
mailing list