search

iLCSoft / SIO

Simple IO package
BSD 3-Clause "New" or "Revised" License
0 stars 9 forks source link

Catch cases where unexpected data is in the stream when dumping #5

Closed tmadlener closed 4 years ago

tmadlener commented 4 years ago

This can be triggered by corrupted files, but it can also be triggered if some information / data has been stored in the stream outside of the usual SIO flow.

BEGINRELEASENOTES

ENDRELEASENOTES

rete commented 4 years ago

This can be triggered by corrupted files, but it can also be triggered if some information / data has been stored in the stream outside of the usual SIO flow.

A corrupted file will most probably have the marker written but rest of data will be corrupted. In this case, the error code won't be no_marker but something else I don't recall.

I suggest a simpler message: "Interrupt: no record marker found".

We could also add, for any other error code, a message like: "ERROR: " + what()

What do you think?

tmadlener commented 4 years ago

I think if we single out the no_marker error code from the others, the printed message should cover all the possibilities in which it can occur. I am not sure if this is something that should or can be done for all other error codes as well. But in general, I agree, that all exceptions should lead to a (somewhat) nicely formatted message and not just a few selected ones.

On a slightly different topic; should it be considered to merge sio-dump and sio-dump-detailed into one executable and offer a -v or --verbose flag to switch between the two behaviors? Otherwise, I think there will be a lot of duplicated code between the two.

rete commented 4 years ago

I think if we single out the no_marker error code from the others, the printed message should cover all the possibilities in which it can occur. I am not sure if this is something that should or can be done for all other error codes as well. But in general, I agree, that all exceptions should lead to a (somewhat) nicely formatted message and not just a few selected ones.

I think the most frequent other one is probably io_failure, though others are also possible... A generic message might be enough for other code than that.

On a slightly different topic; should it be considered to merge sio-dump and sio-dump-detailed into one executable and offer a -v or --verbose flag to switch between the two behaviors? Otherwise, I think there will be a lot of duplicated code between the two.

Sounds like a good idea. Maybe a flag like --detailed sounds better. verboseis generally reserved for logging. Do you want to implement it in this PR? Yes or no are valid answers.

tmadlener commented 4 years ago

Sounds like a good idea. Maybe a flag like --detailed sounds better. verboseis generally reserved for logging. Do you want to implement it in this PR? Yes or no are valid answers.

Yes, I can do that.

rete commented 4 years ago

Looks really cool now. If you are ok I can merge it now

tmadlener commented 4 years ago

Updated the release notes to reflect the latest changes. From my side, I think everything that should be there is there, so I'm OK with merging.