emacs-devel
[Top][All Lists]
Advanced

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

Re: New session management patch.


From: Jan D.
Subject: Re: New session management patch.
Date: Wed, 27 Feb 2002 20:43:47 +0100

Richard Stallman wrote:
> 
>     +/* Return non-zero if PATH is an executable file. */
>     +static int
>     +executable_file_p (path)
>     +     char *path;
> 
> In GNU we don't call these things "paths", we call them file names.
> We use the term "path" only for a list of directories to search.
> Would you please change names accordingly?

This function will go away.  But it was modeled after file_p in xrdb.c, maybe
that should be changed:
  static int
  file_p (path)
       char *path;
  {
    ...

> 
>     +/* Try to find program by searching path from environment.  If not found,
>     +   the CURRENT_DIRECTORY concatenated with PROGRAM is returned.
>     +   The return value is allocated with malloc and the caller must free 
> it. */
>     +static char*
>     +path_search (program, current_directory)
> 
> Why not use openp for this?  Is there some reason you can't?
> 
> But why do you need this anyway?  Can't you get the info
> from Vinvocation_name and Vinvocation_directory?

I didn't know about those.  I will use Vinvocation_name and
Vinvocation_directory instead, it will simplify things.  Do I nead to GCPRO
these before use?  Are the rules for using GCPRO/UNGCPRO written down
somewhere?

> 
>     +  if (strchr(program, PATH_SEP_CHAR) || ! (bp = getenv ("PATH")))
>     +      return make_absolute_path (program, current_directory);
> 
> Please write a space before an open paren after a function name.
> Likewise everywhere else.
> 
> Also, the indentation there is not correct.

I have a conflicting rule at work, so when I switch to Emacs it comes out
wrong.  I will fix this.

> 
>     +      if (proglen+len+1 <= MAXPATHLEN)
>     +        {
>     +          char maybe[MAXPATHLEN+1];
> 
> That is an arbitrary limit, so please remove it.  openp avoids having
> such a limit.

Yes and no as several has pointed out.  But since this will be done in a much
more simpler way, the code will be removed.  MAXPATHLEN is used in other places
in Emacs, like buffer.c and fileio.c, that is why I used it.

        Jan D.



reply via email to

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