[PATCH setxkbmap] Eliminate limitations on path length.

[Van de Bugger]

No. I believe that is wrong approach. I suppose fopen does not crash if
path too long but returns appropriate error, so we just should check
result of fopen carefuly, and issue appropriate error message (using
errno and strerror). 

On Wed, 2011-02-16 at 02:56 +0500, Alexandr Shadchin wrote:
> Also fopen() can not open path greater than PATH_MAX.
> Path still need to verify that it was less than PATH_MAX.
> 
> 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.
> > +*/
> > +
> > +char *
> > +rprintf(char const * format, ...)
> > +{
> > +    va_list args;
> > +    char * buffer = NULL;
> > +    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)
> > +        {
> > +            ERR1("`vsnprintf' returned unexpected error (%d).\n", required);
> > +            abort();
> > +        }
> > +    }
> > +    while (required >= allocated);
> > +    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 +653,6 @@ FILE *
> >  findFileInPath(char *name, char *subdir)
> >  {
> >      register int i;
> > -    char buf[PATH_MAX];
> >      FILE *fp;
> >  
> >      if (name[0] == '/')
> > @@ -635,16 +664,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 +848,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 +875,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
> > 
> > 
> > 
> > _______________________________________________
> > 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
>