Re: [PATCH 6/7] py-minsymbol: Add interface to access minimal_symbols

[Jeff Mahoney <jeffm@suse.com>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2/5/16 11:43 AM, Phil Muldoon wrote:
> On 04/02/16 17:29, jeffm@suse.com wrote:
>> From: Jeff Mahoney <jeffm@suse.com>
>> 
>> This patch adds a new gdb.MinSymbol object to export the
>> minimal_symbol interface.  These objects can be resolved using a
>> new gdb.lookup_minimal_symbol call. --- +static PyObject * 
>> +msympy_get_file_name (PyObject *self, void *closure) +{ +
>> struct minimal_symbol *minsym = NULL; + +  MSYMPY_REQUIRE_VALID
>> (self, minsym); + +  return PyString_FromString
>> (minsym->filename); +}

In writing the test cases, I discovered that this value usually (but
not always) has a value but the value tends to be meaningless or at
least compiler dependent.  In my test program, it dumps asm("...")
code into crtstuff.c.  I tried using an assembly file instead (with
generic pseudoinstructions and labels) and it presented an empty
filename.  When I load up the kernel, it puts every minimal symbol in
csum-wrappers_64.c (at least for this kernel build).  Given the
questionable utility of this value, should I even keep it?

[Removed other details for review; I have addressed them in each patch.]

> The patch series, in principle, looks fine to me other than the
> nits I've highlighted.
> 
> However it needs tests for the new functionality.  It would be
> helpful if you could indicate if you have run the testsuite and
> checked for regressions.
> 
> It also needs documentation additions and changes, and a
> documentation review.
> 
> Also please wait for a maintainer to sign off on it, too

Thanks for the review.  I've added tests and documentation for all but
regcache (which I'm about to do) and it already sussed out several
bugs.  I ended up having to rework the gdb.lookup_symbol for static
symbols patch significantly since it did introduce regressions.  It's
a lot simpler now.  I'll post the updated set once I finish up regcache.

- -Jeff

- -- 
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.19 (Darwin)
Comment: GPGTools - http://gpgtools.org

iQIcBAEBAgAGBQJWuLl7AAoJEB57S2MheeWySVsQAICvXs3bfkMCTXa6pnWHra+z
s+tKiyC6gjOX2U2bDzZq3uxSdbArtB3Rgl4TTRs3BFAAOdVLwsMw0jCEZjaNZQO1
D0qwErfrD0E7RF+YKKslako8+Au+yAMcqlQt97XnQFURSLHQ3Dp/jGyRRH9qSXo4
kFatHeKUlhX8gV2nkc7xwKTddxGnUSSHCb02UWcZOZ7Mjaa4ORVpvmWTHp5KdLWJ
S4AGzR4G/JB90ew31aZriAFqQbQTHMcauOhoMSktLobAMsVRX/zX6/CLXNntOPoT
tI8YK834H+DMGJVYzqOx9uqaxtTo8E84IMVMG4X1XIkgd1au4qQeuEyyfGdwfE0Z
Iun5rgCVaGtg8VxCB66jfDloImlfYbwZgGnN/+oWHlhTHurko5ma7xgV/mDgoUUw
J0v/m0yZ8pmMYtVSK2eWBcrKQcqdoKRa+XGqwa69/vWTwD0LnxUA8tphA61XOn3m
x26tHsokAXGmXy+RBrCMYa+ABehBcB5AQFZYxTHCPFF5qJlJs5MYT29+HdxsS71N
A/iHoJWaPPiM8FqmqLGexnS1rxClwd4DK7hiwq43b5gbfw4axXPp3l6AC1nQ++4M
ZPHz18KmlVRkgiNcAxMXwMCxGW04cbWkD/VrhnMk8TnNWYDWZyabIAXJnKTeJKIU
0Qox9ZHBecYHXtL0ADCu
=1A6j
-----END PGP SIGNATURE-----