pam_radius_auth on Mac OS 10.6.2 - patch attached
Stefan Winter
stefan.winter at restena.lu
Thu Feb 4 08:54:00 CET 2010
Hi,
thanks for the review. I was a bit too hasty yesterday and could have
done better indeed. I meanwhile found gettimeofday(), it merely needs an
own #include. That makes the portions of my patch which deal with
missing gettimeofday() and the RNG problem go away.
The much more trivial patch is appended below.
Greetings,
Stefan Winter
Am 04.02.10 03:42, schrieb Frank Cusack:
> On February 3, 2010 4:24:07 PM +0100 Stefan Winter
> <stefan.winter at restena.lu> wrote:
>> Please review the patch - especially where entropy in the random number
>> generater is reduced.
>
> Pay attention to your comments. It's important to get spellings of
> things like gettimeofday() right even in comments. (The errors in
> english are fine and I'm not criticizing that at all.)
>
> It's bad to use platform-specific defines the way you have. Rather
> than imply that MacOS means no gettimeofday(), test for gettimeofday()
> and your macro should be based on that.
>
> #ifndef HAVE_GETTIMEOFDAY
> ...
> #endif
>
> Worst case, if you can't test for gettimeofday(), your MACOS #define
> should set/clear HAVE_GETTIMEOFDAY. At least then there is only one
> place it would have to be changed in the future.
>
> something.h:
> #ifndef MACOS
> #define HAVE_GETTIMEOFDAY
> #endif
>
> I only reviewed your patch by looking at the patch itself; I didn't
> look at it in context, so this isn't 100% but I think the so-called
> RNG there is pretty ineffective. So I wouldn't complain about your
> change, but yours looks to be really bad! uninitialized is just as
> likely to be a constant value as it is to be a random value.
>
> -frank
> -
> List info/subscribe/unsubscribe? See
> http://www.freeradius.org/list/devel.html
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: pam_radius_auth_mac_v2.patch
URL: <http://lists.freeradius.org/pipermail/freeradius-devel/attachments/20100204/c68e36bc/attachment.ksh>
More information about the Freeradius-Devel
mailing list