Re: Patches for mc-4.5.51



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





[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]