Re: Also handle "set input-radix 0" and "set output-radix 0"

[Pedro Alves <pedro at codesourcery dot com>]

On Tuesday 06 January 2009 18:58:44, Tom Tromey wrote:

> For the output radix, in reality we only support 3 possible values
> (why not binary as well?  I have no idea).

Yes, that's validated already.

> 
> For the input radix... in practice I think the code also really only
> supports bases <= 16.  E.g., c-exp.y:parse_number says:
> 
> 	  if (base > 10 && c >= 'a' && c <= 'f')
> 	    {
> 	      if (found_suffix)
> 		return ERROR;
> 	      n += i = c - 'a' + 10;
> 	    }
> 
> I didn't check the other languages.  However, given the above, I would
> suggest validating the input radix as well.

Yeah, that's actually mentioned in the code already:

static void
set_input_radix_1 (int from_tty, unsigned radix)
{
  /* We don't currently disallow any input radix except 0 or 1, which don't
     make any mathematical sense.  In theory, we can deal with any input
     radix greater than 1, even if we don't have unique digits for every
     value from 0 to radix-1, but in practice we lose on large radix values.
     We should either fix the lossage or restrict the radix range more.
     (FIXME). */

  if (radix < 2)

> 
> Even if we want to support more input radices, there should still be
> some limit, maybe 36 (10 numeric digits plus 26 alphabetic digits).
> FWIW, my preference is not to support more.  They lead to parsing
> weirdness -- is "32u" an unsigned constant 32 in base 36?
>

Yes, but this is independent of the issue I'm focusing on, which
is the fact that we do 0->UINT_MAX translation before reaching the 
set function, which doesn't make any sense in this case, and a few
others I've listed.

The more I think about this, the more I think we should either make
the set function be a real setter --- that is, it should be passed
in the new value as argument, and it should handle the setting itself;
or, split the validation into a new function, and declare that the
current "set" callbacks are post-set callbacks (which is what they
are currently actually).

> I suspect that this kind of validation would mean we would not really
> need var_zuinteger.  I don't feel strongly about this, though, and it
> does express the intent more clearly.

We could get by with var_zinteger in this case, but I also I think
that var_zuinteger makes things clearer, and that it should also
be used in more cases.  var_zinteger should be fixed to print its
value as an integer, instead of doing:

do_setshow_command ()
{
 (...)
	case var_zinteger:
	  if (arg == NULL)
	    error_no_arg (_("integer to set it to."));
	  *(int *) c->var = parse_and_eval_long (arg);
            ^^^^^
 (...)

	  /* else fall through */
	case var_zinteger:
	  fprintf_filtered (stb->stream, "%u", *(unsigned int *) c->var);
                                                 ^^^^^^^^^^^^^^
	  break;
 (...)

-- 
Pedro Alves