Hello Julia,

thanks for your report. Please see my comments below.

I have also quickly examined the source code in your branch and I have a few nits, mostly concerning the coding style. It is nothing critical, but it is perhaps better to fix this early than late, and have it in mind when writing new code:

* Please do not forget to put spaces around binary operators. E.g. type
  "crc >> 8" instead of "crc>>8".

* Likewise, type "uint16_t crc = 0;" instead of "uint16_t crc=0;" and
  "switch (LE16(d->id))" instead of "switch(LE16(d->id))".

* Please declare symbols and functions in header files as extern. It is
  also preferable that you remove argument names from the declarations
  of functions in header files (keep just the types).

* In udf_read_allocation_sequence(): Would it be possible to replace
  the numbers 0, 1, 2, etc. in the switch by symbolic constants? Either
  preprocessor macros or enum values.

* Please get rid of the unnecessary and excessive empty lines at the
  end of the source files and between functions.

In case file, I have to read part of file by position and length. So,
I found sector where my position  locates and read data according
conditions. In case directory, I have number of descriptor (file of
directory) which I have to read. It is means that I have to find FID
(File identifier descriptor). But here I meet some problem. According
to docs one sector could contains one or more FIDs, but I didn't find
any information about could or not sector contains part of FIDs. I
hope not, but I am testing it now.

I think that in situations like this, when you encounter missing or inconclusive documentation, it might be helpful as a last resort to look into the source code of UDF drivers of other open source operating systems. Certainly not to "borrow" the code from them (especially if the code is not BSD!), but to find out how the others have dealt with the missing pieces of the puzzle. (But always try to consult multiple sources, so you don't replicate a bug of one specific implementation by accident.)

Mimicking the behaviour of say Linux or *BSD is far from an ideal solution, but it is still better than just guessing what an acceptable solution might be given the inconclusive documentation.

I will make commit with read functions when I finish testing. I
haven't implemented  match function yet, and It is means that it can't
pass normal tests. And when I was reading sequence of allocation
descriptors I didn't read allocator which redirects to next sequence.
I.e. by this time I read not all files from directory. But I didn't
have any problem with it. I just moved that part of work after reading
to be sure I can interpret allocators.

I admit that I am starting to get lost in the various features you have already implemented and you are about to implement. Would you perhaps create an overview table (on our wiki) which would capture features you are about to implement? For each feature there might be columns it the table such as:

a) Is the feature already implemented in your branch.
b) Is the feature required for the essential functionality of the UDF
   driver (or is it optional). You can even distinguish between various
   versions of the UDF specification.
c) If not implemented yet, then what features this one depends on.


M.D.

_______________________________________________
HelenOS-devel mailing list
[email protected]
http://lists.modry.cz/cgi-bin/listinfo/helenos-devel

Reply via email to