Perfect that has fixed it. I have tested it on my Dev server and it gives
the correct output. Will roll out to one production box during the day and
monitor. Then I will feedback if there are any more problems but it looks
to me like it is working well.
Thank you so much for the help. It is really very much appreciated.
Now that we know it works by which release do you think it will be ready.
On Wed, Nov 7, 2012 at 1:50 AM, Phil Pennock <pdp@???> wrote:
> On 2012-11-06 at 10:32 +0200, James Isolder wrote:
> > I have tried it for both the new code and both still don't output the
> > set_id. I am freebsd 9 if that helps at all.
>
> Ah, no, my mistake. Sorry. :(
>
> I thought the auth vars should only be set if authentication succeeded.
> In fact, they *can* be set even if authentication fails, and it's the
> return code which matters.
>
> The expanded value of `server_set_id` goes into a short-lived variable,
> which is used for logging and diagnostics; _only_ if the authenticator
> also returned `OK` does this value make it into the `$authenticated_id`
> C-level variable.
>
> I must have known this at some point, because the authenticators I've
> written myself do the right thing. *scratches head* Dementia, that
> must be it.
>
> Okay, so new plan:
> ----------------------------8< cut here >8------------------------------
> 201 auth_vars[0] = expand_nstring[1] = msgbuf;
> 202 expand_nlength[1] = Ustrlen(msgbuf);
> 203 expand_nmax = 1;
> 204
> 205 /* clean up globals which aren't referenced, but still shouldn't be
> left
> 206 pointing to stack memory */
> 207 #define CLEANUP_RETURN(Code) do { auth_vars[0] = expand_nstring[1] =
> NULL; \
> 208 expand_nlength[1] = expand_nmax = 0; return (Code); } while (0);
> ----------------------------8< cut here >8------------------------------
>
> Ugh, that trailing semi-colon on line 208 was also dumb.
>
> Change line 201 to:
> ----------------------------8< cut here >8------------------------------
> auth_vars[0] = expand_nstring[1] = string_copy(msgbuf);
> ----------------------------8< cut here >8------------------------------
>
> Change lines 207,208 to:
> ----------------------------8< cut here >8------------------------------
> #define CLEANUP_RETURN(Code) do { return (Code); } while (0)
> ----------------------------8< cut here >8------------------------------
>
> If that works, then the commit will be to rip out the CLEANUP_RETURN()
> calls entirely and use "return" directly, but changing the macro is a
> simpler change for testing.
>
> (You can make this change with or without the change I previously
> requested; worst case scenario is that when auth succeeds, a few more
> bytes are allocated from a pool, to hold the userid twice).
>
> -Phil
>
