[review] Make 'target_{short,long}name' inline functions and check if 'current...

["Andrew Burgess (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>]

Andrew Burgess has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/481
......................................................................


Patch Set 1:

(1 comment)

I haven't had time to play with this code yet, but I had one question...

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/481/1/gdb/target.h 
File gdb/target.h:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/481/1/gdb/target.h@1312 
PS1, Line 1312: 
1307 |   target_ops *m_stack[(int) debug_stratum + 1] {};
1308 | };
1309 | 
1310 | /* The ops structure for our "current" target process.  This should
1311 |    never be NULL.  If there is no target, it points to the dummy_target.  */
1312 > 
1313 | extern target_ops *current_top_target ();
1314 | 
1315 | /* Define easy words for doing these operations on our current target.  */
1316 | 
1317 | static inline const char *

If this is the correct way to resolve this issue - I defer to others with more knowledge in this area - then you should probably change or expand this comment as its clearly not correct, right.  I mean this comment right here is the fundamental cause of your bug, it assumes current_top_target () can never return NULL, and you have found a case when it can.

That said, and I don't know the impact this would have - what if you "just" fixed current_top_target so that it really can never return NULL, but instead forces dummy_target to be returned?



-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I8a9a4dd6edc3784dc089f37d56f2d020567b4aee
Gerrit-Change-Number: 481
Gerrit-PatchSet: 1
Gerrit-Owner: Sergio Durigan Junior <sergiodj@redhat.com>
Gerrit-CC: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Comment-Date: Fri, 01 Nov 2019 08:40:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment