Mercurial > octave
changeset 33240:4e5bc9c4f657
use std::stoi or std::stoul instead of atoi (bug #65342)
The std::stoi and std::stoul functions allow us to distinguish between
invalid values and out of range values instead of just returning 0 for
any conversion failure as atoi does. In the following changes, we now
attempt to provide better diagnostics where possible.
* pathsearch.cc (directory_path::init): Use std::stoul instead of
atoi. Issue warnings for out of range or invalid values.
* __init_fltk__.cc (fltk_uimenu::add_entry): Use std::stoi instead of atoi.
* event-manager.cc (F__event_manager_file_dialog__): Use std::stoi
instead of atoi.
* bp-table.cc (bp_table::parse_dbfunction_params): Use std::stoi
instead of atoi.
* data-conv.cc (oct_data_conv::string_to_data_type): Use std::stoi
instead of atoi.
* debug.cc (parse_start_end, parse_integer_argument): New static functions.
(Fdbtype): Use parse_start_end to improve parsing of integer arguments.
(Fdblist, Fdbstack, do_dbupdown): Use parse_integer_argument to
improve handling of integer argument values.
| author | John W. Eaton <jwe@octave.org> |
|---|---|
| date | Sat, 23 Mar 2024 12:13:17 -0400 |
| parents | 775dde0cb3e5 |
| children | 39b6d6ca3831 432e0151f652 |
| files | libinterp/corefcn/debug.cc libinterp/corefcn/event-manager.cc libinterp/dldfcn/__init_fltk__.cc libinterp/parse-tree/bp-table.cc liboctave/util/data-conv.cc liboctave/util/pathsearch.cc |
| diffstat | 6 files changed, 247 insertions(+), 93 deletions(-) [+] |
line wrap: on
line diff
--- a/libinterp/corefcn/debug.cc Fri Mar 22 23:51:33 2024 -0400 +++ b/libinterp/corefcn/debug.cc Sat Mar 23 12:13:17 2024 -0400 @@ -592,6 +592,69 @@ return ovl (); } +static bool +parse_start_end (const std::string& arg, int& start, int& end, const char *who) +{ + start = 0; + end = 0; + + std::size_t ind = arg.find (':'); + + if (ind != std::string::npos) // (start:end) + { + std::string start_str = arg.substr (0, ind); + std::string end_str = arg.substr (ind+1); + + try + { + start = std::stoi (start_str); + + if (end_str == "end") + end = std::numeric_limits<int>::max (); + else + end = std::stoi (end_str); + } + catch (const std::invalid_argument&) + { + error ("%s: invalid integer conversion while parsing range '%s'", who, arg.c_str ()); + } + catch (const std::out_of_range&) + { + error ("%s: integer value out of bounds while parsing range '%s'", who, arg.c_str ()); + } + + if (std::min (start, end) <= 0) + error ("%s: start and end lines must be >= 1\n", who); + + if (start > end) + error ("%s: start line must be less than end line\n", who); + } + else // (dbtype lineno) + { + try + { + int line = std::stoi (arg); + + if (line <= 0) + error ("%s: start and end lines must be >= 1\n", who); + + start = line; + end = line; + } + catch (const std::invalid_argument&) + { + // May be a name instead of a number. + return false; + } + catch (const std::out_of_range&) + { + error ("%s: integer value out of bounds while parsing '%s'", who, arg.c_str ()); + } + } + + return true; +} + DEFMETHOD (dbtype, interp, args, , doc: /* -*- texinfo -*- @deftypefn {} {} dbtype @@ -633,40 +696,8 @@ { std::string arg = argv[1]; - std::size_t ind = arg.find (':'); - - if (ind != std::string::npos) // (dbtype start:end) - { - std::string start_str = arg.substr (0, ind); - std::string end_str = arg.substr (ind + 1); - - start = atoi (start_str.c_str ()); - if (end_str == "end") - end = std::numeric_limits<int>::max (); - else - end = atoi (end_str.c_str ()); - - if (std::min (start, end) <= 0) - error ("dbtype: start and end lines must be >= 1\n"); - - if (start > end) - error ("dbtype: start line must be less than end line\n"); - } - else // (dbtype fcn) || (dbtype lineno) - { - int line = atoi (arg.c_str ()); - - if (line == 0) // (dbtype fcn) - fcn_name = arg; - else // (dbtype lineno) - { - if (line <= 0) - error ("dbtype: start and end lines must be >= 1\n"); - - start = line; - end = line; - } - } + if (! parse_start_end (arg, start, end, "dbtype")) + fcn_name = arg; } break; @@ -674,31 +705,8 @@ { fcn_name = argv[1]; - std::string arg = argv[2]; - std::size_t ind = arg.find (':'); - - if (ind != std::string::npos) - { - std::string start_str = arg.substr (0, ind); - std::string end_str = arg.substr (ind + 1); - - start = atoi (start_str.c_str ()); - if (end_str == "end") - end = std::numeric_limits<int>::max (); - else - end = atoi (end_str.c_str ()); - } - else - { - start = atoi (arg.c_str ()); - end = start; - } - - if (std::min (start, end) <= 0) - error ("dbtype: start and end lines must be >= 1\n"); - - if (start > end) - error ("dbtype: start line must be less than end line\n"); + if (! parse_start_end (argv[2], start, end, "dbtype")) + error ("dbtype: expecting start:end or location argument, found '%s'", argv[2].c_str ()); } break; @@ -725,6 +733,27 @@ return ovl (); } +static int +parse_integer_argument (const std::string& arg, const char *who) +{ + int n = 0; + + try + { + n = std::stoi (arg); + } + catch (const std::invalid_argument&) + { + error ("%s: invalid value of N, found '%s'", arg.c_str (), who); + } + catch (const std::out_of_range&) + { + error ("%s: value of N ('%s') is out of range", arg.c_str (), who); + } + + return n; +} + DEFMETHOD (dblist, interp, args, , doc: /* -*- texinfo -*- @deftypefn {} {} dblist @@ -748,11 +777,7 @@ octave_value arg = args(0); if (arg.is_string ()) - { - std::string s_arg = arg.string_value (); - - n = atoi (s_arg.c_str ()); - } + n = parse_integer_argument (arg.string_value (), "dblist"); else n = args(0).int_value (); @@ -798,7 +823,7 @@ if (s_arg == "-completenames") continue; - n = atoi (s_arg.c_str ()); + n = parse_integer_argument (s_arg, "dbstack"); } else n = arg.int_value (); @@ -944,11 +969,7 @@ octave_value arg = args(0); if (arg.is_string ()) - { - std::string s_arg = arg.string_value (); - - n = atoi (s_arg.c_str ()); - } + n = parse_integer_argument (arg.string_value (), who.c_str ()); else n = args(0).int_value (); } @@ -1038,10 +1059,10 @@ n = -2; else { - n = atoi (arg.c_str ()); + n = parse_integer_argument (arg, "dbstep"); if (n < 1) - error ("dbstep: invalid argument"); + error ("dbstep: N must be greater than zero"); } } else
--- a/libinterp/corefcn/event-manager.cc Fri Mar 22 23:51:33 2024 -0400 +++ b/libinterp/corefcn/event-manager.cc Sat Mar 23 12:13:17 2024 -0400 @@ -362,7 +362,19 @@ if (idx != 2) retval(idx++) = str; else - retval(idx++) = atoi (str.c_str ()); + { + // FIXME: Should we warn or error on invalid or out of + // range values in STR? When atoi was used for + // conversion instead of std::stoi we did not. Was + // that intentional? + + try + { + retval(idx++) = std::stoi (str); + } + catch (const std::invalid_argument&) { } + catch (const std::out_of_range&) { } + } } } } @@ -377,7 +389,22 @@ for (int idx = 0; idx < nel; idx++, it++) items.xelem (idx) = *it; - retval = ovl (items, *it++, atoi (it->c_str ())); + auto fpath = *it++; + + int idx = 0; + + // FIXME: Should we warn or error on invalid or out of range + // values in *IT? When atoi was used for conversion instead of + // std::stoi we did not. Was that intentional? + + try + { + idx = std::stoi (*it); + } + catch (const std::invalid_argument&) { } + catch (const std::out_of_range&) { } + + retval = ovl (items, fpath, idx); } return retval;
--- a/libinterp/dldfcn/__init_fltk__.cc Fri Mar 22 23:51:33 2024 -0400 +++ b/libinterp/dldfcn/__init_fltk__.cc Sat Mar 23 12:13:17 2024 -0400 @@ -603,9 +603,21 @@ { std::string valstr = fltk_label.substr (idx1 + 1, len - 1); fltk_label.erase (idx1, len + 1); - val = atoi (valstr.c_str ()); - if (val > 0 && val < 99) - val++; + + // FIXME: Should we warn or error on invalid or out + // of range values in VALSTR? When atoi was used + // for conversion instead of std::stoi we did not. + // Was that intentional? + + try + { + val = std::stoi (valstr); + + if (val > 0 && val < 99) + val++; + } + catch (const std::invalid_argument&) { } + catch (const std::out_of_range&) { } } std::ostringstream valstream; valstream << val;
--- a/libinterp/parse-tree/bp-table.cc Fri Mar 22 23:51:33 2024 -0400 +++ b/libinterp/parse-tree/bp-table.cc Sat Mar 23 12:13:17 2024 -0400 @@ -370,6 +370,9 @@ // allow "in" and "at" to be implicit if (args(pos).is_string ()) { + // Default value. + tok = dbstop_in; + std::string arg = args(pos).string_value (); if (arg == "in") { @@ -386,10 +389,16 @@ tok = dbstop_if; pos++; } - else if (atoi (args(pos).string_value ().c_str ()) > 0) - tok = dbstop_at; else - tok = dbstop_in; + { + try + { + if (std::stoi (args(pos).string_value ()) > 0) + tok = dbstop_at; + } + catch (const std::invalid_argument&) { } + catch (const std::out_of_range&) { } + } } else tok = dbstop_at; @@ -431,8 +440,27 @@ // FIXME: we really want to distinguish number // vs. method name here. - if (atoi (arg.c_str ()) == 0) + // FIXME: I'm not sure what the + + bool int_conv_ok = true; + + try + { + if (std::stoi (arg) == 0) + int_conv_ok = false; + } + catch (const std::invalid_argument&) { + int_conv_ok = false; + } + catch (const std::out_of_range&) + { + int_conv_ok = false; + } + + if (! int_conv_ok) + { + // Assume we are looking at a function name. // We have class and function names but already // stored the class name in fcn_name. class_name = fcn_name; @@ -458,12 +486,27 @@ { if (args(pos).is_string ()) { - int line = atoi (args(pos).string_value ().c_str ()); + bool skip = false; + + std::string str = args(pos).string_value (); + + try + { + int line = std::stoi (str); - if (line > 0) - lines.insert (line); - else - break; // may be "if" or a method name + if (line > 0) + lines.insert (line); + else + break; // may be "if" or a method name + } + catch (const std::invalid_argument&) + { + break; // may be "if" or a method name + } + catch (const std::out_of_range&) + { + error ("dbstop: location value out of range '%s'", str.c_str ()); + } } else if (args(pos).isnumeric ()) {
--- a/liboctave/util/data-conv.cc Fri Mar 22 23:51:33 2024 -0400 +++ b/liboctave/util/data-conv.cc Sat Mar 23 12:13:17 2024 -0400 @@ -391,8 +391,21 @@ { if (s[pos] == '*') { - block_size = atoi (s.c_str ()); - s = s.substr (pos+1); + try + { + block_size = std::stoi (s); + s = s.substr (pos+1); + } + catch (const std::invalid_argument&) + { + (*current_liboctave_error_handler) + ("invalid repeat count in '%s'", s.c_str ()); + } + catch (const std::out_of_range&) + { + (*current_liboctave_error_handler) + ("repeat count out of range in '%s'", s.c_str ()); + } } else (*current_liboctave_error_handler) @@ -458,8 +471,21 @@ { if (s[pos] == '*') { - block_size = atoi (s.c_str ()); - s = s.substr (pos+1); + try + { + block_size = std::stoi (s); + s = s.substr (pos+1); + } + catch (const std::invalid_argument&) + { + (*current_liboctave_error_handler) + ("invalid repeat count in '%s'", s.c_str ()); + } + catch (const std::out_of_range&) + { + (*current_liboctave_error_handler) + ("repeat count out of range in '%s'", s.c_str ()); + } } else (*current_liboctave_error_handler)
--- a/liboctave/util/pathsearch.cc Fri Mar 22 23:51:33 2024 -0400 +++ b/liboctave/util/pathsearch.cc Sat Mar 23 12:13:17 2024 -0400 @@ -108,10 +108,35 @@ if (! octave_kpse_initialized) { - std::string val = sys::env::getenv ("KPATHSEA_DEBUG"); + std::string env_val = sys::env::getenv ("KPATHSEA_DEBUG"); + + if (! env_val.empty ()) + { + unsigned int env_debug_flags = 0; + + try + { + unsigned long val = std::stoul (env_val); - if (! val.empty ()) - kpse_debug |= atoi (val.c_str ()); + if (val > std::numeric_limits<unsigned int>::max ()) + (*current_liboctave_warning_with_id_handler) + ("Octave:kpathsea-debug-value-ignored", "directory_path::init: ignoring out of range KPATHSEA_DEBUG value '%s'", env_val.c_str ()); + else + env_debug_flags = val; + } + catch (const std::invalid_argument&) + { + (*current_liboctave_warning_with_id_handler) + ("Octave:kpathsea-debug-value-ignored", "directory_path::init: ignoring invalid KPATHSEA_DEBUG value '%s'", env_val.c_str ()); + } + catch (const std::out_of_range&) + { + (*current_liboctave_warning_with_id_handler) + ("Octave:kpathsea-debug-value-ignored", "directory_path::init: ignoring out of range KPATHSEA_DEBUG value '%s'", env_val.c_str ()); + } + + kpse_debug |= env_debug_flags; + } octave_kpse_initialized = true; }