InsightSoftwareConsortium / ITK

Insight Toolkit (ITK) -- Official Repository. ITK builds on a proven, spatially-oriented architecture for processing, segmentation, and registration of scientific images in two, three, or more dimensions.
https://itk.org
Apache License 2.0
1.37k stars 661 forks source link

Wrap lowlevel c++ string conversion exceptions with ITK exceptions. #3213

Open hjmjohnson opened 2 years ago

hjmjohnson commented 2 years ago

Description

ITK was recently changed from using 'ato[ifd]' to using 'sto[ifd]' in an attempt to provide more robust error checking.

commit b9a384af33e904ae0da212ace9130632c808c6fd
Author: Hans Johnson <hans-johnson@uiowa.edu>
Date:   Sat Sep 22 12:57:23 2018

    STYLE: Prefer error checked std::stoi over atoi

    The atoi function does not provide mechanisms for distinguishing between
    '0' and the error condion where the input can not be converted.

    std::stoi provides exception handling and detects when an invalid
    string attempts to be converted to an integer.

    atoi()
      Con: No error handling.
      Con: Handle neither hexadecimal nor octal.

    The use of atoi in code can cause it to be subtly broken.
    atoi makes two very big assumptions indeed:
      The string represents an integer/floating point value.
      The integer can fit into an int.

Impact analysis

The above change now introduces a data dependant throw of low-level C++ exceptions. ITK should convert these into ITK exceptions instead, with non-generic messages based on the situation.

Expected behavior

ITK generates context-specific ITK exceptions in data-induced exceptions.

Actual behavior

Default low level c++ exeptions are thrown of type

If no conversion could be performed, an [invalid_argument](https://www.cplusplus.com/invalid_argument) exception is thrown.

If the value read is out of the range of representable values by an int, an [out_of_range](https://www.cplusplus.com/out_of_range) exception is thrown.

Versions

All TIK versions after commit b9a384af33e904ae0da212ace9130632c808c6fd

Additional Information

This change was discussed in the context of : https://github.com/Slicer/Slicer/pull/6200

hjmjohnson commented 2 years ago

@kian-weimer This issue would be a really nice improvement to ITK that you could take on after the wrapping stuff is figured out better.