Re: Patches for mc-4.5.51

[Jindrich Makovicka <makovick kmlinux fjfi cvut cz>]

Pavel Roskin wrote:
> On Sat, 20 Sep 2003, Jindrich Makovicka wrote:
>
>
> > Fr?d?ric L. W. Meunier wrote:
> >
> > > Anyway, if I was a programmer I'd contribute to HEAD, not fork
> > > and have my ideas used by a few people.
> >
> > Here is my favourite one, resume support for ftp (& others), I am using
> > it for about a year, and don't think anymore that this feature will ever
> > get into "official" mc :)
>
>
> I don't see any comments about this code.  I don't understand the changes
> in vfs/local.c.  Why do we seek the file only if it's in the linear mode?

Just to make sure the patch won't break the functionality of mc and to
make the vfs open functions behave the same way as direntry.c:vfs_s_open.

> I think there are no limitations for seek on local files.  I understand
> setting O_LINEAR by the caller of mc_open means certain limitations of
> what the caller can do.  I don't think it has means that VFS may not seek
> the file.  I would expect you to change to comment in vfs.h if you change
> the semantic of that flag.
>
> Oh, I see it now.  You arranged the code so that offset will be 0 if
> O_LINEAR is not set.  Looks like a trap for those who will work on this
> code after you.

I don't understand. It doesn't change any semantics, except that with
O_LINEAR, an argument for offset is read. That's all.

> Also, there is a warning in vfs.c:
>
> vfs.c: In function `mc_open':
> vfs.c:388: warning: `mode' might be used uninitialized in this function
>
> As far as I can see, the warning is genuine.  "mode" and "offset" are used
> in the same call.  Can it be happen that the file you are going to reget
> disappears at this point, and VFS creates a world writable file?

Yes, they are, but only with O_CREAT. The if condition can be eventually
left out, if you don't like it, but you'll have to supply a dummy mode
argument when opening an existing file with O_LINEAR.

> mc_open() is supposed to be a VFS equivalent of open().  I don't feel good
> about adding the fourth argument to it.
>
> What could you cay about following comment from vfs.h?
>
>  *      c) lseek() immediately after open(), giving ftpfs chance to
>  *         reget. Be warned that this lseek() can fail, and you _have_
>  *         to handle that gratefully.
>
> If that doesn't work, let's create new function mc_open_linear() with 4
> arguments and get rid of O_LINEAR.

Sounds easier. moreover, most of the functionality is already
implemented in vfs_s_open, there's just no way to pass the offset
argument. Actually, I just changed the remaining vfs open functions to
match vfs_s_open.

> Liberal development doesn't mean that patches that make the code more
> fragile are welcome.

Once more: This patch shouldn't change any mc_open semantics, except
when using O_LINEAR, which is used only in one place.

Regards,
--
Jindrich Makovicka