find_and_open_source bug?

find_and_open_source bug?

[Doug Evans]

If find_and_open_source opens the file via rewrite_source_path,
should xfullname get called?
Grep for XXX below.

Seems so, otherwise the caller can't really assume what's
stored in *fullname is a "fullpath" (as defined by xfullpath).
[Maybe rewritten source paths shouldn't get passed through xfullpath
but then why does the rest of the function do that?]

[I'm trying to understand what invariants callers of find_and_open_source
can expect.]

int
find_and_open_source (const char *filename,
		      const char *dirname,
		      char **fullname)
{
  char *path = source_path;
  const char *p;
  int result;

  /* Quick way out if we already know its full name.  */

  if (*fullname)
    {
      /* The user may have requested that source paths be rewritten
         according to substitution rules he provided.  If a substitution
         rule applies to this path, then apply it.  */
      char *rewritten_fullname = rewrite_source_path (*fullname);

      if (rewritten_fullname != NULL)
        {
          xfree (*fullname);
          *fullname = rewritten_fullname;
        }

      result = open (*fullname, OPEN_MODE);
      if (result >= 0)
        XXX shouldn't we pass *fullname through xfullpath here?
	return result;
      /* Didn't work -- free old one, try again.  */
      xfree (*fullname);
      *fullname = NULL;
    }
[...]

Re: find_and_open_source bug?

[Tom Tromey]

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> If find_and_open_source opens the file via rewrite_source_path,
Doug> should xfullname get called?
Doug> Grep for XXX below.

Doug> Seems so, otherwise the caller can't really assume what's
Doug> stored in *fullname is a "fullpath" (as defined by xfullpath).
Doug> [Maybe rewritten source paths shouldn't get passed through xfullpath
Doug> but then why does the rest of the function do that?]

Doug> [I'm trying to understand what invariants callers of find_and_open_source
Doug> can expect.]

Probably nobody has ever tried to do something weird here.

I think it would be safe for you to pick an approach that preserves
whatever sensible user scenarios there are.  E.g., require that path
substitutions map full paths to full paths or something like that.

Tom