Re: New #mtools extfs?

[Pavel Roskin <proski gnu org>]

Hello!

> I found faster to remove the archive existence check (there is no need
> for it. In fact the extfs script does this also for sure if it really
> needs the archive) from the extfs.c:217 (CVS). There might also be a
> check for the filesystem name if desired (like: if the fsname ==
> 'mtools' then go on).

I don't understand why you need to disable this check.  I don't think it's 
right to reuse the archive filename as the disk name.  Actually, what I 
suggested was "/#dos:a/" - this syntax doesn't need your patch.

>      if (uses_archive){
>         if (mc_stat (name, &mystat) == -1)
> -           return NULL;
> +           ;

I don't like this coding style.  mystat is actually used later.  If you
ignore the return value of mc_stat, then current_archive->extfsstat will
point to invalid data.

In fact, mystat is allocated on the stack, so current_archive->extfsstat 
becomes invalid in any case as soon as open_archive() returns.  
current_archive is allocated by g_new() and is returned by open_archive() 
using the pparc argument.  I'll have to debug mc to understand why it 
doesn't crash later.

The code is ugly already, let's not make is even worse.

> I also patched the vfc.c so that now it works like this: "X: -> 
> /X#mtools". The only changes in the C code is the mentioned check and 
> modified vfs_translate_url() as you suggested. The script needed some 
> more changes since the archive argument was added.

I'm not going to apply it in this form.  Do it in the right way, i.e. by 
using flags after ":", like it's done in fish.

> Here it works pretty good. What do you think? I can send you a complete 
> patch against the CVS if this principal is acceptable for you.

I'll appreciate if you do it correctly and comment your changes, so that 
they can be understood without looking at the code all the time.

> > What if we implement something like this:
> > 
> >   VFS name                mtools/dos name
> > 
> > /#dos:a/                  a:\
> > /#dos:c/dos/sys.com       c:\dos\sys.com

Please quote only relevant parts of the messages you are replying to.  
You didn't consider my suggestion and didn't even explain the reason.

-- 
Regards,
Pavel Roskin