How to get an error code from iso_stream_read() ?

[scdbackup]

My experimental update DVD-R produces the first bad spot after 20 sessions. That should not be overly dramatic in this use case, but an ABORT event sneaked in.

This is because i naively put the return value of iso_stream_read() into iso_error_to_msg() and iso_error_get_severity() which yields "Unknown error" and "ABORT". That return value is -1.

I would count an unreadable sector as "FAILURE", maybe even as "FATAL" if the read occasion is essential to libisofs. But -1 is obviously not the libisofs error code to look at.

So how do i learn about the error details after iso_stream_read() failed ?

[vreixo]

Iteresenting question, that turns out to be a little bug. The question is that iso_stream_read(), when used to read from a disc, delegates in IsoDataSource?->read_block(). If that fails, it returns < 0, and libisofs returns the same error code to the user, so (s)he must check the return codes of his IsoDataSource? implementation. As you're probably using the libisoburn IsoDataSource? implementation (libisoburn/data_source.c) you should check their return codes (in fact, currently it does not specify any return code, just -1 on error!).

Some, preliminary diagnosys: it is not a libisofs problem. And it may be a libisoburn one. And the response to your question would be

So how do i learn about the error details after iso_stream_read() failed ?

Check the IsoDataSource? implementation.

Ok? Actually no. Because iso_stream_read() can also return a libisofs error, so we have an ugly mess. Ugly? No really, as I have already thought about that, and that is the reason for the bits 19-16 of my error codes. Obviously, we forgot about it, both in IsoDataSource? implementation and API documentation. I mean, all libisofs interfaces you implement outside libisofs (IsoFilesystem?, IsoDataSource?, ...) should return either a valid libisofs error code, or a code that follows libisofs encoding of error codes, and whose bits 19-16 are different than 0.

So I'd like to introduce my proposal again: what about using the same error scheme in all libburnia libs?

[scdbackup]

I think the correct way would be if the libisofs layer generates a libisofs error when it encounters an error from the underlying read function (which through libisoburn called a read function in libburn which either uses MMC commands or POSIX functions).

[scdbackup]

The behavior of libisoburn data source complies to this prescription in libisfs-0.6.2 API:

    /**
     * Read an arbitrary block (2048 bytes) of data from the source.
     *
     * @param lba
     *     Block to be read.
     * @param buffer
     *     Buffer where the data will be written. It should have at least
     *     2048 bytes.
     * @return
     *      1 if success, < 0 on error
     */
    int (*read_block)(IsoDataSource *src, uint32_t lba, uint8_t *buffer);

So if you want to blame the data source then you have to break the API specifications first.

You cannot rely on getting any other information than "<0" from a IsoDataSource. There is no guarantee to have a libburn behind the reader function. Anything can generate the data and events of that outlet.

[vreixo]

I think the correct way would be if the libisofs layer generates a libisofs error when it encounters an error from the underlying read function

This is not a good idea, as all possible errors will become a generic READ_ERROR or similar. I consider much more valuable to pass the underlying error to the user.

you have to break the API specifications first.

maybe you are right. The afirmation above assumes the returned error follow libisofs error specification. But unfortunatelly that is not documented, so breaking the API is the best alternative.

[scdbackup]

Breaking seems not needed.

The API bible says

struct iso_data_source
{

    /* reserved for future usage, set to 0 */
    int version;
...

So you may introduce version==1 and prescribe that its read_block() has to issue error codes out of a list which is published in the API.

For version==0 i still propose to return ISO_FILE_READ_ERROR or something other than -1.

[vreixo]

So you may introduce version==1 ... For version==0 i still propose to return ISO_FILE_READ_ERROR or something other than -1.

That is exactly what I don't want, because it is a workaround that forces us to deal with two different situations on our code. And that problem is not exclusive of IsoDataSource?. The lack of documentation, or wrong documentation, can be an issue in several functions.

At the end there is the discussion "API stability" vs "clean and manteinable code". When a library is mature enought and there are lots of apps using it, API stability is very important. However, in early stages of development, a clean and manteinable code is much more important. That is the current stage of libisofs. At this time, I only know about an app based on it (xorriso), and this little error question is surely not a problem for it.

That is the reason I had say that guarantee API stability from 0.6.2 was not a good idea. When API can be kept stable at no cost (for example, using getters/setters instead of direct access to estructure fields), it is really valuable. However, when we have to introduce those ugly things, for me API stability is not important (for now, of course, it will be on the future, for example after 1.0 with UDF, filters, etc)

So at least my development branch will break API. If somebody wants to take care of mantaining API stability, it is ok for me to keep it in mainline. I just have no interest no time to mantain it at the price of uglier code.

I've chosen to implement ng libisofs to avoid the manteinance of the ugly old code. What I won't do now it to have the same problems in ng!

[pygi]

Then the documentation should be revised, and improved, don't you think?

What about API stability & clean code? It exists. It's not a tabu or result of extra-terrestrial activity. I've seen it before, I've done it. Libburn is quite young, it is not perfect, but we have managed to keep merry quite clean codebase with stable API. If I remember right, the only time we broke api was because of a serious bug that we could not workaround with something that would be at least mildly clean. My memory might serve me wrong at this time however. Workaround on workaround is ugly. One workaround now with prescription that this api may break in say 0.9.x release cycle isn't forbidden. We will probably break API in 0.9.x in order to prepare for 1.0 release which will then be maintained while we work on 1.5.x series which will eventually lead to 2.0. But those changes should be minimal, and should mainly be cleanup of the workarounds we accumulated up to that point. I got requests for libisofs every day. I accumulate them in my head, and think how will I implement them *while* not breaking API. It's a matter of smart design.

Please, everyone should restrain themselves from saying deterministic things like "my branch will break API", or I'll do that or that. We will do everything together, and we will try to think of a smart way to do it. That's the only possibly way anyway.

[vreixo]

Documentation fixed in my mainline, revision 379. I'll mark this bug as fixed with that. You are free to re-open it if you think we must provide another fix to keep API stable, but I won't take further actions on this.

[scdbackup]

bzr bundle to adjust level3 version of libisofs to the needs of libisoburn/xorriso-0.2.5

[scdbackup]

I added four error codes for use with iso_data_source.read() which say "Read error occured with IsoDataSource?" with severities SORRY, MISHAP, FAILURE, or FATAL.

libisoburn-0.2.5 will use them in its own data source implementation.

[vreixo]

Patch accepted and merged to mainline.