Re: [Gnu-arch-users] [BUG] my-revision-library broken

[Johannes Berg]

On Tue, 2004-09-28 at 12:18 -0400, Aaron Bentley wrote:
> It looks like you're stumbling over this:
> 181: dir = directory_as_cwd (argv[1]);

I think I'm looking at a totally different copy of the code. Here, I
have a copy that doesn't use the dir variable (in my.c,
arch_set_my_library_path)

> When adding revision libraries, it's important to canonicalize the 
> directory name.  Otherwise, you can wind up with two copies of the same 
> library, and things just get ugly.  With the current code, the following 
> are equivalent:
> 
> "tla my-revision-library ~/foo"
> "tla my-revision-library ~//foo"
> "cd; my-revision-library foo"
> "cd ~/foo; my-revision-library ."
> 
> IMHO, this is as it should be.

Sure, agreed!

> Furthermore, it doesn't make sense to add a revision library that 
> doesn't exist.  Yet it certainly makes sense to remove one that doesn't 
> exist.

The latter is what I'm stumbling over.

> So, I think the appropriate code is:
> 
> if (dir_exists (dir))
>    dir = canonicalize (dir);
> else if (operation == add)
>    printf ("Can't add revlib; dir does not exist\n%s: ", dir);
>    exit (2);
> 
> Does that help?

Thats about what I was thinking, but the code in my.c is somewhat more
complex since it rebuilds the complete list.

> > Could someone else please look at it? Whoever
> > does can also remove the dir variable from
> > libarch/my.c:arch_set_my_library_path(), it is apparently unnecessary.
> 
> Huh?  That function requires a dir to add/remove.

Here, it has a local variable "dir", and its only uses are:
  dir = safe_current_working_directory ();
  lim_free (0, dir);


johannes

signature.asc
Description: This is a digitally signed message part