[Freeipmi-devel] Re: Fujitsu's iRMC S2 - support for "get SEL entry long text" within ipmi-oem

[Dan Lukes]

On 08/17/10 23:10, Albert Chu:
> Attached is my reworked patch for ipmi-oem.  Can you give it a shot and
> LMK if it works?

Done. Some notes:

1. --------------
if user call for record id >= 65536 the number record ID is silently 
changed to something else. I recommend not to allow out-of-range values 
at all.

2. --------------
> bytes_rq[8] = UCHAR_MAX;

Requests with MaxResponseDataSize>100 will fail.
Also response packet can't be larger than IPMI_OEM_MAX_BYTES

So you can't ask for more than min(IPMI_OEM_MAX_BYTES-17,100) bytes.

3. --------------
> component_length = (rs_len - 16);

I prefer more safe approach - I assume that string may end before packet 
end (e.g. len is obtained by strlen). And I doesn't assume that string 
is '\0' terminated in response also.

The so-called documentation from Fujitsu is so vague to be trusted so 
much, IMHO ...

4. ----------------
... in advance, you count the length including the trailing '\0' (which 
is present in every response - not only the last part of string), so the 
concatenation doesn't work (off-by-one bug)

5. ----------------
>  while (offset < data_length)

... will be neverending loop when
IPMI_OEM_FUJITSU_SEL_ENTRY_LONG_TEXT_MAX_STRING_LENGTH < data_length

You should replace
offset = IPMI_OEM_FUJITSU_SEL_ENTRY_LONG_TEXT_MAX_STRING_LENGTH
with
offset = data_length
to resolve the issue. Or use "break;" ...

6. ----------------
bytes_rq array need not to be intialized every round (only offset is 
changing). Also, most of results needs to be extracted from bytes_rs 
only once.

7. ----------------
If user asked for 'first' or 'last' record then use actual_record id for 
subsequent (looped) requests to obtain consistent results as last SEL 
may become non-last in meantime.

 ======================

> I did (atleast try) to implement the code to loop if there was more text
> to retrieve.  Hopefully I implemented it right.

See [4], [5] and [7]

> Also, I slightly tweaked the output to more similarly match ipmi-sel
> output, adding in semi-colons and stuff.  I admit, I'm not entirely sure
> of the output.  You may have specifically chosen your output

I tried to follow output format of ipmi-sel.

I attached the another iteration of patch.

                                Dan

patch-DAN-ipmi-some_fujitsu_extension
Description: Text document