emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: emacs & MAXPATHLEN


From: Eli Zaretskii
Subject: Re: emacs & MAXPATHLEN
Date: Fri, 29 Jul 2005 16:54:41 +0300

> From: Giuseppe Scrivano <address@hidden>
> Date: Fri, 29 Jul 2005 02:22:37 +0200
> Cc: address@hidden
> 
> This little patch should fix it.

Thanks, but please see my comments below.

> @@ -5146,14 +5146,47 @@
>        && stat (".", &dotstat) == 0
>        && dotstat.st_ino == pwdstat.st_ino
>        && dotstat.st_dev == pwdstat.st_dev
> -      && strlen (pwd) < MAXPATHLEN)
> -    strcpy (buf, pwd);
> -#ifdef HAVE_GETCWD
> -  else if (getcwd (buf, MAXPATHLEN+1) == 0)
> -    fatal ("`getcwd' failed: %s\n", strerror (errno));
> +#ifdef MAXPATHLEN
> +      && strlen (pwd) < MAXPATHLEN
> +#endif
> +      )
> +    {
> +      buf = malloc(strlen(pwd)+1);
> +      if(!buf)
> +        fatal ("`malloc' failed in init_buffer\n");

This should have used xmalloc instead of calling malloc and checking
for errors.

> +#ifdef _GNU_SOURCE
> +  else
> +    {
> +      buf = get_current_dir_name();
> +      if(!buf)
> +        fatal ("`get_current_dir_name' failed in init_buffer\n");
> +    }

I think this part is unnecessary.  The old code didn't use
get_current_dir_name, so I think we should not introduce it now.

In any case, if we do introduce get_current_dir_name, we should add an
Autoconf test for it, and then use HAVE_GET_CURRENT_DIR instead of
_GNU_SOURCE (which I think is the wrong test anyway, btw: _GNU_SOURCE
has nothing to do with availability of certain library functions).

> +          if(getcdwd (buf, MAXPATHLEN+1) == 0)
                ^^^^^^^
There's a typo in this line.  Did you check that all the branches
actually compile?

> +  else
> +    {
> +      buf = malloc(MAXPATHLEN+1);

You are using MAXPATHLEN without testing for it being defined.  If it
is okay to assume that it is always defined, then why you introduced a
test for that in the first hunk of your changes (see above)?

> +      if(!buf)
> +        fatal ("`malloc' failed in init_buffer\n");

Again, please use xmalloc.

> +      buf = malloc(MAXPATHLEN+1);
> +      if(!buf)
> +        fatal ("`malloc' failed in init_buffer\n");
> +      if(buf)
> +        {
> +          if(getwd (buf) == 0)
> +            fatal ("`getwd' failed: %s\n", buf);
> +        }

And here too: please use xmalloc, and please resolve the MAXPATHLEN
issue.

> +    }
>  #endif
> 
>  #ifndef VMS
> @@ -5189,6 +5222,7 @@
> 
>    temp = get_minibuffer (0);
>    XBUFFER (temp)->directory = current_buffer->directory;
> +  free(buf);
>  }
> 
>  /* initialize the buffer routines */
> 
> 
> 
> I worked on the xsmfns.c file too. I am not sure if this is required for the 
> hurd compatibility.  

The same comments apply there as well.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]