Re: New #mtools extfs?

[Standa Opichal <opichals seznam cz>]

Hi!

On Thu, 5 Dec 2002, Pavel Roskin wrote:

> There is another solution - make drive part of the filename:
> /#dos:/a/

I don't see where can I get the /a/ part within the script. Can you help 
me here with some more specific code suggestion?

> Then current_archive->extfsstat will still have random data.  Random data
> is bad.  Sometimes is cannot be distinguished from valid data.

Yes, then I'll set it all to 0. The extfsstat.st_nlink != 0 should than be 
tested before used. This would clearly say that the contents is invalid.

> > > current_archive->extfsstat becomes invalid in any case as soon as
...
> Right.  It's an implicit copy of the whole structure.  It's used very
> rarely.  I figured it out as soon as I sent my previous e-mail.

Hmm, I just figured out that it is in fact _not_ used anywhere in the 
code. It is simply optimized out because all usage is beginning with a 
line like this:

if (vfs_uid && (!(s->st_mode & 0004)))
    ...

The vfs_uid is '#define vfs_uid 0'... according to the vfs/ChangeLog it 
was you who set it to this...
--- snip ---
2002-11-03  Pavel Roskin  <proski gnu org>
        * vfs.h: Define vfs_uid and vfs_gid as 0, they never change.
--- end ---

Actually the lines like above were used to explicit permission check on 
the archive. I didn't checked what it was before your patch, but this 
seems really strange to me. Could you explain it somehow?

> Let's distinguish two things:
> 
> 1) if stat() is not needed at all, then don't call it and remove all the
> code that deals with that data.  Bigger patches are harder to understand,
> but introducing dead variables of fields is not better.

It the current CVS version state the stat() is _not_ needed at all. The 
vfs_uid == 0 and therefore the extfsstat is not used at all.

> 2) if stat() is not needed only for mtools, then let's not call stat() for
> mtools (and fill mystat with zeroes), although I would rather avoid this
> special case.

I Agree.

I propose to investigate why you did the change on 2002-11-03 for '#define 
vfs_uid 0'. In any case the extfsstat is only used for explicit permission 
checks on the archive file entry. Currently these checks are out of order 
so if it all works as needed we can remove all the checks from the code 
IMHO. I can't see a reason for these in any of the vfs filesystem. The 
permission check is done by other functions used for the archive handling.
Tip: try 'grep vfs_gid *' or 'grep vfs_uid *' in the CVS/vfs folder to see 
where it is used.

> > > 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.
> 
> Maybe using the disk as part of the path can make it unnecessary.  If this
> approach doesn't work,

I can't see what way do you mean (as I've writen above)...

> then indeed we need to pass the flags, probably in the environment.

One extra command line argument would not hurt I think for the 'list' 
command...

> > Actually, in the case of this patch (disable the stat() result check)  
> > appliance, I can't see any reason to have the ':' flags in the extfs.ini
> > configuration. If the archive does exist then this will be ok and the
> > stat() information will be valid. Otherwise either the script would fail
> > (e.g. on unzip -Z archiveName) because of that the archive doesn't exist
> > or the script (like my #mtools) would use the archive name for something
> > different or not use this at all. I see this as a matter of
> > simplification rather than making code more unreadable. Sure I can
> > comment the changes more to be more selfdescriptive.
> 
> OK, let's do the simplification first.  It's really worth it to clean up
> the code before trying to fix anything non-trivial in that code.

You didn't explicitely agreed the part above about the removal of the ':' 
flag within the .ini file. Could you confirm this? It is maybe a subject 
for another mail here, but it is related in my eyes a bit.

> It's more a matter of time than preference.  I wish I could dig into the
> recent stderr redirection problems right now, but it already 4am here, and
> I do have a job (no, it's not hacking mc).

Keep alive! ;)

> That's fine.  If you care about mtools support, then please continue.

Now its your turn again. ;)

best regards

STan