[PR 17] GRETH: Fixed autonego timeout overflow problem

[Daniel Hellstrom]

Hello,

Ralf and Thomas, you are correct. This patch was created a long time ago when 'struct rtems_clock_time_value' was used it was defined:

typedef struct {
   /** This is the seconds portion of a time of day. */
   uint32_t    seconds;
   /** This is the microseconds portion of a time of day. */
   uint32_t    microseconds;
} rtems_clock_time_value;

Joel removed the structure in c5fd2cb6746ab2af2f38d74b17db07d39e6fb97a and 57674ef62e38b6c5566ea4c47fcd616aadb3ee5b, and introduced 'struct timeval' instead which is defined with signed tv_usec 
instead of unsigned microseconds. I have missed this during merging somewhere long time ago.

Joel, can you ignore this patch and close the PR1465. I will update the spreadsheet.

Thanks for reviewing,
Daniel


On 03/20/2012 08:51 AM, Daniel Hellstrom wrote:
> Hello Ralf,
>
> The problem was first reported by Joris, please see BUG 1465.
>
> -    msecs = (tnow.tv_sec-tstart.tv_sec)*1000+(tnow.tv_usec-tstart.tv_usec)/1000;
> +   msecs = (int)(tnow.tv_sec-tstart.tv_sec)*1000+((int)tnow.tv_usec-(int)tstart.tv_usec)/1000;
>
> The seconds part is no problem. If (tstart.tv_usec > tnow.tv_usec) will be negative, but since the variables are unsigned a underflow will result in a very long msecs timeout.
>
> Thanks,
> Daniel
>
>
> On 03/19/2012 11:26 PM, Ralf Corsepius wrote:
>> On 03/19/2012 08:59 PM, Joel Sherrill wrote:
>>> This looks ok to me.
>>
>> I am having strong doubts on this patch.
>>
>> Daniel, could you elaborate the problem you are trying to solve?
>>
>> Ralf
>>
>>
>>> I would have thought we had some support routines to do this but alas.
>>>
>>> Please commit someone.
>>> ________________________________________
>>> From: rtems-devel-bounces at rtems.org [rtems-devel-bounces at rtems.org] On Behalf Of Daniel Hellstrom [daniel at gaisler.com]
>>> Sent: Monday, February 06, 2012 8:15 AM
>>> To: rtems-devel at rtems.org
>>> Subject: [PR 17] GRETH: Fixed autonego timeout overflow problem
>>>
>>> Signed-off-by: Daniel Hellstrom<daniel at gaisler.com>
>>> ---
>>>   c/src/libchip/network/greth.c |    2 +-
>>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/c/src/libchip/network/greth.c b/c/src/libchip/network/greth.c
>>> index aff4d0f..b0e70b4 100644
>>> --- a/c/src/libchip/network/greth.c
>>> +++ b/c/src/libchip/network/greth.c
>>> @@ -339,7 +339,7 @@ greth_initialize_hardware (struct greth_softc *sc)
>>>               while (!(((phystatus = read_mii(phyaddr, 1))>>  5)&  1)) {
>>>                       if ( rtems_clock_get_tod_timeval(&tnow) != RTEMS_SUCCESSFUL )
>>>                         printk("rtems_clock_get_tod_timeval failed\n\r");
>>> -                    msecs = (tnow.tv_sec-tstart.tv_sec)*1000+(tnow.tv_usec-tstart.tv_usec)/1000;
>>> +                    msecs = (int)(tnow.tv_sec-tstart.tv_sec)*1000+((int)tnow.tv_usec-(int)tstart.tv_usec)/1000;
>>>                       if ( msecs>  GRETH_AUTONEGO_TIMEOUT_MS ){
>>>                               sc->auto_neg_time = msecs;
>>>                               sc->auto_neg = -1; /* Failed */
>>> -- 
>>> 1.7.0.4
>>>
>>> _______________________________________________
>>> rtems-devel mailing list
>>> rtems-devel at rtems.org
>>> http://www.rtems.org/mailman/listinfo/rtems-devel
>>>
>>> _______________________________________________
>>> rtems-devel mailing list
>>> rtems-devel at rtems.org
>>> http://www.rtems.org/mailman/listinfo/rtems-devel
>>
>>
>
> _______________________________________________
> rtems-devel mailing list
> rtems-devel at rtems.org
> http://www.rtems.org/mailman/listinfo/rtems-devel
>
>