ifc icon indicating copy to clipboard operation
ifc copied to clipboard

Try fixing exceptions stuff accordingly to comments in #30 and #36

Open menuet opened this issue 2 years ago • 2 comments

Try a minimal fix:

  • In struct InvalidPartitionName, replace member std::string_view name by char[NAME_BUFFER_SIZE] name and use a class-static make function to initialize the buffer
  • In Reader::Reader, replace if (not ifc.header()) throw "file not found"; by assert(ifc.hader());
  • In translate_exception(), catch the exceptions of type InvalidPartitionName and std::exception

menuet avatar Oct 27 '23 19:10 menuet

Hi I think I did all the suggested corrections (or gave justification for not doing them). Is there a chance that this PR is accepted? It seems not very risky and it fixes a real bug, so even if my proposed PR is not perfect, I think that it would be beneficial for anyone willing to try the project.

menuet avatar Nov 03 '23 20:11 menuet

Hi I think I did all the suggested corrections (or gave justification for not doing them). Is there a chance that this PR is accepted? It seems not very risky and it fixes a real bug, so even if my proposed PR is not perfect, I think that it would be beneficial for anyone willing to try the project.

Indeed, I think the fixes here are worth evaluating. I have been talking with @GabrielDosReis and we plan on reviewing the changes here sometime later this week or early next week--apologies for the delays.

Gaby is unsure about the static buffer change in the exception object, but dynamic memory via something like std::string is also a no-go. The original design had it be a std::string_view so that the string lifetime was governed by the system attempting to process the bad partition so having the exception object effectively duplicate the string is where we're left scratching our heads. We should have a more decided answer for you soon.

cdacamar avatar Nov 06 '23 16:11 cdacamar