epics-base / pvDataCPP

pvDataCPP is an EPICS V4 C++ module
https://epics-base.github.io/pvDataCPP/
Other
6 stars 16 forks source link

Mismatched docs/implementation for getField #66

Closed brunoseivam closed 5 years ago

brunoseivam commented 5 years ago

The documentation for Structure::getField(size_t) and Union::getField(size_t) claim that they return a null pointer if the index is not valid. The implementation throws a out_of_range exception instead (from vector::at).

Also, Union::getField(size_t) returns FieldConstPtr but Structure::getField(size_t) returns const FieldConstPtr &. I think both should return FieldConstPtr for consistency.

Structure:

https://github.com/epics-base/pvDataCPP/blob/cd2436342d71b173eb4377e61d396e348c15546a/src/pv/pvIntrospect.h#L734-L740

Union:

https://github.com/epics-base/pvDataCPP/blob/cd2436342d71b173eb4377e61d396e348c15546a/src/pv/pvIntrospect.h#L857-L863

brunoseivam commented 5 years ago

Potentially fixed via #67

mdavidsaver commented 5 years ago

Looking back, it seems that Structure::getField(size_t) has never returned NULL.

It's original form in 2012 (87bff33c302a16ff4b4716fd6d5538a9e10af7c7) was

FieldConstPtr getField(std::size_t index) {return fields[index];}

And it is now:

const FieldConstPtr& getField(std::size_t index) const {return fields.at(index);}

So originally, out of bounds access was simply undefined (probably SIGSEGV). Now it throws an exception.