pam_radius_auth on Mac OS 10.6.2 - patch attached
Hi, to my own surprise, I got pam_radius_auth 1.3.17 compiled and running on Mac OS X 10.6.2. It needed some minor source code changes and guesswork in the Makefile. I've attached a patch which shows the difference. The new Makefile will then have a new target make mac Please review the patch - especially where entropy in the random number generater is reduced. Greetings, Stefan Winter
On February 3, 2010 4:24:07 PM +0100 Stefan Winter <stefan.winter@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
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@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
Stefan Winter wrote:
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.
Why not: #ifdef __APPLE__ ? That would be easier. :) Alan DeKok.
Hi, if only I'd known that this flag exists :-) Here you go, another try attached. Stefan Am 04.02.10 09:23, schrieb Alan DeKok:
Stefan Winter wrote:
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.
Why not:
#ifdef __APPLE__
?
That would be easier. :)
Alan DeKok. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
looks pretty great to me. On February 4, 2010 9:46:27 AM +0100 Stefan Winter <stefan.winter@restena.lu> wrote:
Hi,
if only I'd known that this flag exists :-)
Here you go, another try attached.
Stefan
Am 04.02.10 09:23, schrieb Alan DeKok:
Stefan Winter wrote:
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.
Why not:
# ifdef __APPLE__
?
That would be easier. :)
Alan DeKok. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
participants (3)
-
Alan DeKok -
Frank Cusack -
Stefan Winter