Re: Unsecure assoc calls (was: Make default margin values depend on paper size)

[Carl Sorensen]

On 9/11/09 3:52 AM, "Michael Käppler" <address@hidden> wrote:

> Hi Carl,
> (was it your intention to discuss this off-list? I ask because you
> didn't cc the list for the last two times.)

No, I made a mistake the first time, and the second time I replied all to
your email that replied to mine.

>> Hmmm -- I see your point.
>> 
>> Without looking into the code for each of those grep results, I couldn't
>> answer.  But assoc-get is defined in LilyPond, and was certainly recommended
>> by Han-Wen when I first started programming Scheme in LilyPOnd.
>> 
>> I wonder if I'm being overly sensitive, or if many of these assoc calls are
>> left over from before assoc-get was defined.
> I would propose to modify assoc-get the way it is indicated in the
> attached patch and then migrate all assoc calls to assoc-get calls.
> This assoc-get calls would be divided into two classes.
> 1. Silently set a default value, since there is nothing erroneous happening.
> 2. Set a default value and output a programming_error, showing the key
> which failed.

I think this is a good idea, because it gives us an additional tool to
cleanly handle errors.

> LY_DEFINE (ly_assoc_get, "ly:assoc-get",
>           2, 1, 0,

This line is wrong; you need to have the total of these three numbers
be the total number of arguments.

If you change this to 2, 2, 0, then strict-checking will be an optional
argument, and all of the existing calls will work as is.

If you don't want to have strict-checking an optional argument, then the
line should be 3, 1, 0, and strict-checking will have to come before
default_value, as default_value is an optional argument.


> -          (SCM key, SCM alist, SCM default_value),
> +          (SCM key, SCM alist, SCM default_value, SCM strict_checking),
>           "Return value if @var{key} in @var{alist}, else
> @code{default-value}"
>           " (or @code{#f} if not specified).")
> 

IIRC, programming errors are not internationalized.  Is that your
understanding?


> Of course that would also happen with chain-assoc-get.
> I also recognized that some assoc-get or chain-assoc-get calls occur
> with #f set as default value, which I think is unnecessary since #f is
> the hardcoded default already.

It's probably not necessary, but if you really want the default value of #f,
it's probably best to include it in the code.  It conveys the intent more
clearly to the reader, and doesn't cost anything in performance, IMO.

Thanks,

Carl