New #mtools extfs?
Standa Opichal
opichals at seznam.cz
Wed Dec 4 08:26:00 UTC 2002
Hi!
On Wed, 4 Dec 2002, Pavel Roskin wrote:
> 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.
Yes, it does although need quite more extensive patch that would send the
part after colon to the extfs script. I'm telling you that this was the
most least affecting patch I can think of...
> 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.
No, it is _not_ a pointer it is only a structure that is copied into
current_archive->extfsstat. Actually if the filesystem is marked so that
it doesn't need an archive name (the 'a' mtools extfs you have in CVS)
than the current_archive->extfsstat would contain the same (possibly
invalid) data as for the case of invalid (nonexistent) archive filename.
> 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.
Nope. It should not crash ... it is not a pointer ... look into the code
more carefuly.
> The code is ugly already, let's not make is even worse.
I would like to not to make it worse. I just see this as very easy
implementation where the archive stat() is not needed at all. It just
prevents the extfs script to be called in case the archive name is not
valid. But the script (or the apps called from in there) do these checks
too, don't they?
> 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.
Hmm, so should I change the extfs.c so that it would send some extra
parameters into the script along the list command? Or what way would you
like it to be done?
> > > 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.
I did, but as I've written for the second time (just look through my
mails) it is not that extfs.c sends any attributes to the script along 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.
If you don't like to dig into the code then leave it to others so they
can judge the patch effect. It seems to me that it is not me who did not
play with the code. Please, take this only as a suggestions and thoughts
that are emerging in my head. I appreciate your effort on mc... I intend
_not_ to offense anyone, I just read the code and see this as a good, not
harming, change.
_best_ regards
STan
More information about the mc-devel
mailing list