On 15 September 2013 12:42, Martin Decky <[email protected]> wrote:
> Hi Jiri,
>
>
> It has taken me a bit longer than I expected, but the current state of
>> my repository is that it can be merged (modulo possible bugs), and
>> further additions won't have so much impact on the rest of the system as
>> they had until now.
>>
>
> Thanks for your email. I am sorry to disappoint, but in my opinion the
> branch is still far from being mergable into mainline. A few reasons for
> that:
>
> * I consider your vcl_* API more like a joke. You replace an easily
> recognizable API (that is, of course, not 100 % standard compliant, but
> still close enough for most practical purposes) with a totally alien API,
> with absolutely no benefits to the user. The client-observable behaviour is
> mostly the same, it still uses a file handle, only strangely packed in a
> structure.
>
>
<rant>
Easily recognizable API? If there is a joke, it's what libc is doing to
POSIX API. At least I do not pretend to be POSIX. You just took a
recognizable interface and purposelessly broke it for every standard C
program.
</rant>
Now to address your other reservations. POSIX API does not consider
separation of names and identities. For POSIX, a name is what the file it.
Not so in my branch. POSIX API is only usable as a kind of very crippled
wrapper, nothing else. By the way, for most practical purposes, you should
use <stdio.h> anyway.
As you noticed, currently, the structure only wraps the file handle. This
change is very recent and made in preparation for client storing much more
information. The client will become a little fatter, and the library itself
is intended to do much more than the VFS server.
[...] but we are also not afraid of reusing existing concepts and APIs
> where it is appropriate.
>
POSIX API is not appropriate here. Not by a long shot.
> * Consistency: To avoid unnecessary confusion, we try to keep the client
> names the same as the server names. We don't have LSCL ("Location Service
> Client Library") and so on, therefore I see no reason for a VCL ("VFS
> Client Library").
>
The actual reason for that is symbol clashes with the server code. You
can't use vfs_ prefix without a lot of conflicts. That is the price for
using C.
> Also, why is the vcl_* stuff as the only thing in the "helenos"
> subdirectory of the libc?
Because nobody else has thought of moving non-standard HelenOS specific
functions into a separate directory, yet.
> And as a nit, I don't consider the "inbox" name very fitting.
>
Propose a better one.
>
> * Coding style: Major issues. You do not respect the 80 characters per
> line rule, some definitions of macro constants are mixed with functions,
> some files are without copyright headers, some header files contain
> external symbol declarations without the "extern" keyword and with argument
> names, there are some really dense functions with no horizontal separators
> (statfs()).
>
>
Some of these issues can be addressed, but for example leaving out argument
names in function declarations is the worst idea ever to enter programming
world. How the hell am I supposed to remember arguments of every function
in the library? I certainly don't want to spend time searching the
directory structure for the implementation, I want to look at the header
and know how to use it.
As for statfs(), I just merged it from the mainline yesterday night.
> * Integration: The "inbox" idea is interesting, but while you do it, why
> not extend it further to non-file objects as well? We still lack the
> equivalent of environment variables and this might be a handy way for
> pointing the spawned task also to other resources (e.g. the compositor in
> case of GUI tasks, etc.).
>
>
Wasn't it you who very specifically made it clear that combining files with
non-file objects is what you want to avoid like plague? Sure you can extend
a lot of it to non-file objects, but it's not something I would do now.
> * VFS server: I fail to see the reason for the pipes. What does it do,
> actually? If it is really just a producer/consumer, we already have a
> generic support for that in libc (adt/prodcons.h).
>
>
It's a pipe. What could it possibly do?
Maybe you are confused by the fact that the library call to MKPIPE is
missing.
> * I might be mistaken, but you though away some functionality from the
> mainline (like in the cat command or the lack of a VFS test) for no obvious
> reason.
>
VFS test is removed now simply because it tested the old API. It makes
little sense to write tests for API that may change or be rejected
completely.
I removed stdin input from cat because currently, it's useless, and would
require additional effort to make work properly. If I am mistaken and
someone actually has figured out a way to use it meaningfully without shell
support for pipes, I will reimplement it promptly.
> * As a bottom line: I fail to see the reason for your comment in "kill"
> and "killall" that the commands should be integrated into a shell. In my
> opinion it is the other way around, many current internal bdsh commands
> should live as separate binaries. They are currently in bdsh only to save
> some footprint until the dynamic linking is fully supported.
>
"kill" and "killall" programs have no value outside user shell. This is
probably a matter of personal taste, but the way processes are managed by
user is subject to shell implementation, and putting shell-specific
operations into stand-alone programs creates clutter in the system.
I can imagine many ways a shell could make it possible for user to
terminate misbehaving programs. "kill" command is one of the uglier things
I can think of.
>
> To sum up, it is good to see where your code is going, but I would really
> prefer to keep this outside the mainline branch for now.
>
>
>
You do understand that with all the response I am getting for various
proposals here (which is, next to none), keeping this outside the mainline
means more private changes I get absolutely no feedback on. Unless you guys
actually participate in external branches, doing any work on HelenOS short
of complete fork is pointless.
You could have subscribed to my branch more than a month ago and dispute my
changes as I was making them. Instead, you wait for me to say "I am happy
with this" just so that you could say you don't like half of it. Not
helping.
-- Jirka Z.
_______________________________________________
HelenOS-devel mailing list
[email protected]
http://lists.modry.cz/listinfo/helenos-devel
