Hello Anton, On Mon, 2008-12-08 at 11:10 +0000, Anton Pak wrote: > Hello! > > Yes, all places you have pointed may produce marshalling assert. > I've meditated a bit and have found no more suspicious API and data > structures. > > Is there any documentation that defines plug-in MAY not fill these fields? >
Yes. SAF HPI spec says the same. 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'. In other words, if the IsSupported is set to 'False', any values assigned to 'Value' and 'Type' fields will be ignored by the HPI user and it does not makes sense. These (not used) fields needs to set to overcome the marshaling code limitation. The same argument holds good for other data structure (listed in below mail chain). Regards, PG > Anton Pak > > > On Mon, 08 Dec 2008 13:59:53 +0300, Raghavendra PG <[EMAIL PROTECTED]> > wrote: > > > 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 > > > > ------------------------------------------------------------------------------ > 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
