Hello Anton, On Fri, 2008-12-05 at 11:36 +0000, Anton Pak wrote: > Hello! > > There are more places where SaHpiSensorReadingT is used and may produce > marshalling asserts: > > SaHpiSensorThresholdsT -> saHpiSensorThresholdsGet/Set() > SaHpiSensorRangeT -> saHpiRdrGet*() > SaHpiSensorEventT -> saHpiEventLogEntryAdd(), saHpiEventGet(), > saHpiEventAdd() >
Thanks for pointing out more functions. The marshalling code will not assert for saHpiSensorThresholdsSet, as the same is taken care in oh_client.cpp (line no. 1933). > I guess, from methodological point of view, better way is to make > plug-ins' code safe. I'm missing something here. Are you suggesting to fix in plugin code or in infrastructure code? > If plug-in returns filled union data structure, it shall always set Type > field. > What say? > If the IsSupported flag is set to 'False', then plugin MAY not fill the Type (SaHpiSensorReadingTypeT) and Value (SaHpiSensorReadingUnionT) fields with proper values. Below is the list of structures which can have (valid) partially filled information: 1. SaHpiSensorReadingT - If IsSupported is set to 'False', then Value and Type fields need not be filled by the plugin. 2. SaHpiSensorThresholdsT - Some or all the fields of this structure may not be filled by the plugin depending on the IsAccessible flag and read/write thresholds in SaHpiSensorThdDefnT structure. 3. SaHpiSensorRangeT - Depending the sensor range flag, some of the fields may not be filled by the plugin. 4. SaHpiSensorEventT - Based on the optional data presentation field, trigger reading and trigger threshold fields may not be filled by the plugin. Below are the functions which has one (or more) structures (listed above) as OUT parameter (or as one of the fields of the OUT parameter structure) 1. saHpiSensorReadingGet 2. saHpiSensorThresholdsGet 3. saHpiEventGet 4. saHpiEventLogEntryGet 5. saHpiRdrGet 6. saHpiRdrGetByInstrumentId These fixes needs to be done in the safhpi.c file. Below are the functions which has one (or more) structures (listed above) as IN parameter (or as one of the fields of the OUT parameter structure) 1. saHpiEventLogEntryAdd 2. saHpiEventAdd These fixes needs to be done in the oh_client.cpp file. If I have missed any other functions/structures, please let me know. Regards, PG > > Anton Pak > > On Fri, 05 Dec 2008 13:52:38 +0300, Ganesha, Raghavendra Pandimakki > <[EMAIL PROTECTED]> wrote: > > > Hello, > > > > Please find the attached patch for correcting this problem. > > If the IsSupported is equal to 'False', then Type and Value will be set > > to zero in saHpiSensorReadingGet() function. > > > > Kindly review. > > > > Regards, > > PG > > > > ________________________________ > > From: Peter D Phan [mailto:[EMAIL PROTECTED] > > Sent: Wednesday, December 03, 2008 8:44 PM > > To: [email protected] > > Subject: Re: [Openhpi-devel] Marshalling asserts for SAF API > > saHpiSensorReadingGet. > > > > > > PG, > > > > We need to correct marshalling code because that is the right place to > > fix this problem. Option 1 is good to have so that each plugin can keep > > the infrastructure code to hurt itself. so if you wish you, or the > > plugin owner, can implement it for OA SOAP. We did that for snmp_bc > > plugin. But we can not count on all the plugins, especially future > > ones, to set Type and Value when IsSupported == FALSE. > > > > > > Regards, > > -------------------------------------------------------------------------------------------- > > P. D. Phan > > IBM Austin > > -------------------------------------------------------------------------------------------- > > > > [cid:477065010@05122008-00EF]Raghavendra PG ---12/03/2008 03:34:26 > > AM---Hello, > > > > > > Raghavendra PG <[EMAIL PROTECTED]> > > > > 12/03/2008 03:34 AM > > Please respond to > > [email protected] > > > > > > > > > > To > > > > "[email protected]" > > <[email protected]> > > > > cc > > > > > > > > Subject > > > > [Openhpi-devel] Marshalling asserts for SAF API saHpiSensorReadingGet. > > > > > > > > Hello, > > > > The openhpi daemon is crashing when trying to get the sensor reading on > > a sensor which is not supporting the reading (IsSupported is set to > > False). > > > > This problem is observed with OA SOAP plugin. We are implementing new > > set of operational sensors whose reading is not supported. > > > > According the SAF HPI spec B.02 section 7.2.5, if the sensor does not > > support reading, then saHpiSensorReadingGet() should set the IsSupported > > field in the 'Reading' structure to 'False' (2nd para in Remarks). This > > means that saHpiSensorReadingGet() API is not setting (changing) the > > other fields of 'Reading' structure by openhpi plugin. The other fields > > are important only if the IsSupported is set to 'True'. > > > > The marshalling code (marshal.c:477) is asserting when it is trying to > > marshal the 1st out parameter (SAHPI_INOUT SaHpiSensorReadingT *Reading) > > of SAF API saHpiSensorReadingGet. > > > > The SaHpiSensorReadingT structure has 3 fields. > > typedef struct { > > SaHpiBoolT IsSupported; > > SaHpiSensorReadingTypeT Type; > > SaHpiSensorReadingUnionT Value; > > } SaHpiSensorReadingT; > > > > If I'm wrong in understanding the marshalling, please correct me. > > > > The assertion is happening when marshalling tried on 3rd field > > (SaHpiSensorReadingUnionT Value) of SaHpiSensorReadingT structure. When > > IsSupported is set to False, the 'Type' and 'Value' are not set. These > > fields will have some junk data. When marshal tries to marshal union and > > it is not able to find the correct value. Hence, marshal code asserts. > > > > This is a limitation in marshalling code. It expects all the fields of a > > return structure to have proper values. > > > > POSSIBLE WORKAROUND: > > --------------------- > > 1. The plugin should set 'Type' and 'Value' fields of the 'Reading' > > structure to some value (can be memset) even when IsSupported is set to > > 'False'. > > All the plugins needs to (which supports sensor without reading support) > > implement this approach > > > > 2. Set all fields of the Reading structure to zero (using memset) before > > calling the plugin ABI oh_get_sensor_reading. > > The plugins need not worry about other fields if the IsSupported is set > > to 'False'. > > > > I'm more inclined towards option 1. > > > > Any suggestions, thoughts or other options? > > > > Regards, > > PG > > > > > > ------------------------------------------------------------------------- > > This SF.Net email is sponsored by the Moblin Your Move Developer's > > challenge > > Build the coolest Linux based applications with Moblin SDK & win great > > prizes > > Grand prize is a trip for two to an Open Source event anywhere in the > > world > > http://moblin-contest.org/redirect.php?banner_id=100&url=/ > > _______________________________________________ > > Openhpi-devel mailing list > > [email protected] > > https://lists.sourceforge.net/lists/listinfo/openhpi-devel > > > > > > ------------------------------------------------------------------------------ > SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada. > The future of the web can't happen without you. Join us at MIX09 to help > pave the way to the Next Web now. Learn more and register at > http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/ > _______________________________________________ > Openhpi-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/openhpi-devel ------------------------------------------------------------------------------ SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada. The future of the web can't happen without you. Join us at MIX09 to help pave the way to the Next Web now. Learn more and register at http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/ _______________________________________________ Openhpi-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openhpi-devel
