Re: url-retrieve may cause hang

[Chong Yidong]

Richard Stallman <address@hidden> writes:

>     I made "status" an extra argument at the beginning of the argument
>     list (so if CBARGS has N elements, the callback is called with N+1
>     arguments).  I described this in the docstring of url-retrieve in my
>     patch below (not yet committed).
>
> Is this new argument _unconditionally_ present?

>From my reading of the code, it is unconditionally present (though it
can be an empty list).

> I can't tell, and I see comments that seem to suggest
> that it might not be so:
>
> +       (setf (car url-callback-arguments)
> +         (nconc (list :error (list 'error 'connection-failed why
> +                                   :host (url-host url-current-object)
> +                                   :service (url-port url-current-object)))
> +                (car url-callback-arguments)))

The car of url-callback-arguments is the new STATUS argument, so this
is correct.

> And this:
>
>       It is called as (apply CALLBACK STATUS CBARGS), where STATUS
>     ! is a list with an even number of elements representing what happened
>     ! during the request, with most recent events first.  Each pair is one
>     ! of:
>     ! 
>     ! \(:redirect REDIRECTED-TO) - the request was redirected to this URL
>     ! \(:error (ERROR-SYMBOL . DATA)) - an error occurred.  The error can be
>     ! signaled with (signal ERROR-SYMBOL DATA).

This is pretty clear: it says there is a STATUS argument; it
definitely doesn't say that argument can be elided (and it isn't).

> I don't like that variability, and it is easy to avoid.  If we are to
> change the API of these callbacks now, let's change it to something
> more consistent: add a single argument unconditionally.  That added
> argument can be a property list in which :redirect and :error may
> occur.

I don't see anything wrong with the patch.  I suggest checking it in
and beginning the pretest.  OK?