On 09/15/2013 02:24 PM, Jiří Zárevúcky wrote: > On 15 September 2013 12:42, Martin Decky <[email protected] > <mailto:[email protected]>> wrote: > > 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:
For the record, I was watching the changes closely until about two weeks or so ago and I quite liked them. The whole series starts with a nice cleanup/handlification of the interface and the code and it seemed reasonable to me. Now, I still need to look at what was done in the most recent changes. What I had reservations against and discussed it with Jiri was the tendency to introduce unnecessarily sophisticated (and thus complex) lockless synchronization in places where a simple mutex or an rwlock would do. I also tried the branch and from the basic functional point of view, it builds ok on all archs and does not have problems running in 32/64-bits or big/little endian configurations. > * 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. Well, we perceived the fact that our low-level filesystem API reminded POSIX and was even using POSIX names as a possible source of confusion ourselves, didn't we? The new API deviates from POSIX even further, for example by removing any predefined meaning from the first three file handles, so some renaming seems definitely justified here. The question is whether it should be vcl_*, but on the other hand, why not? > 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. Martin has a good point in his reply. HelenOS cannot consider itself non-standard and a second-class citizen of its own libc, segregating itself into some HelenOS-specific subdirectory. > * 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. It was probably me, but it was meant in another context: something along the lines that VFS should stay focused on file systems and not somehow integrate the functionality of the location service. On the other hand, even if the concept of inobxes can be extended for other stuff, I don't think it represents a problem. Their use can be extended anytime later. > * 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't see why we are discussing this topic here at this moment and why the branch added a comment of this sort (i.e. quite off-topic) in the first place. The more focused the changes are the better. On a related note, in your recent changes, you added a TODO to replace the restarting automaton which canonifies paths with simpler code. To me, this is a sign of getting distracted from your original goal and gradually trying to rewrite everything, subject to personal taste, of course. Either the automaton does not work and you really cannot figure out how to fix it or it works. In the former case, I would expect a patch replacing it with simpler code which is easier to grasp and fix. In the latter case, why bother? > 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. As I said, I have been following your branch and providing feedback as time permitted. I am now going to have a look at the rest of it. In general, I would not be so strictly against merging this in the near future (unless there is something sinister found in the rest of the series). Of course, it would help to get most of the essential missing functionality back in the meantime and to test/fix the libposix use cases. Jakub _______________________________________________ HelenOS-devel mailing list [email protected] http://lists.modry.cz/listinfo/helenos-devel
