Re: New session management patch.

["Jan D." <jan.djarv@mbox200.swipnet.se>]

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.

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel