Jim,

see attached diff v2.

--Duncan

On Thu, Apr 19, 2012 at 7:49 PM, Jim Mankovich <jm...@hp.com> wrote:

>  Duncan,
>
> The changes you have made look correct to me and I would agree that using
> a function or even a
> #define for the UID verification would be a good idea given there are 4
> places where the exact
> same test is done.  I also think the error message could be made better by
> printing the limits being
> enforced instead of just saying "Invalid user ID: %s"
>
> -- Jim Mankovich | jm...@hp.com --
>
>
> On 4/18/2012 11:02 PM, Duncan Idaho wrote:
>
> On Wed, Apr 18, 2012 at 8:47 PM, Duncan Idaho <dune.id...@gmail.com>wrote:
>
>> Hello all,
>>
>> I gave it some thought, and since IPMI specification is quite exact what
>> range of UIDs is allowed, it seems to be as a good idea to put in more
>> limitations what could and should be accepted as a "valid" UID. In other
>> words, what's missing from 'lib/ipmi_user.c' are couple of ``if (uid >
>> 63)''.
>> Only con I can think of is a] IPMI implementation that violates IPMI spec
>> b] a new spec which would allow UIDs > 63, although this seems unlikely.
>> And since it is noted even in source code itself, I see no problem to
>> "expand" the limits when such time comes.
>>
>> Come to think of it, how about implementation as:
>> ~~~
>> #define UID_MIN 1
>> #define UID_MAX 63
>>
>> [...]
>>
>> some_function()
>> {
>>   [...]
>>   if (uid < UID_MIN || uid > UID_MAX) {
>>     /* error */
>>   }
>>   [...]
>> }
>> ~~~
>>
>> Regards,
>> Duncan
>>
>
> Please, review attached diff.
>
> --Duncan
>
>
> ------------------------------------------------------------------------------
> For Developers, A Lot Can Happen In A Second.
> Boundary is the first to Know...and Tell You.
> Monitor Your Applications in Ultra-Fine Resolution. Try it 
> FREE!http://p.sf.net/sfu/Boundary-d2dvs2
>
>
>
> _______________________________________________
> Ipmitool-devel mailing 
> listIpmitool-devel@lists.sourceforge.nethttps://lists.sourceforge.net/lists/listinfo/ipmitool-devel
>
>
>
> ------------------------------------------------------------------------------
> For Developers, A Lot Can Happen In A Second.
> Boundary is the first to Know...and Tell You.
> Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
> http://p.sf.net/sfu/Boundary-d2dvs2
> _______________________________________________
> Ipmitool-devel mailing list
> Ipmitool-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
>
>

Attachment: 0002-ipmi_user.c-uid_checks-v2.diff
Description: Binary data

------------------------------------------------------------------------------
For Developers, A Lot Can Happen In A Second.
Boundary is the first to Know...and Tell You.
Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
http://p.sf.net/sfu/Boundary-d2dvs2
_______________________________________________
Ipmitool-devel mailing list
Ipmitool-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel

Reply via email to