[PATCH 4/6] Split event_location into subclasses
Tom Tromey
tom@tromey.com
Tue Jan 18 16:59:19 GMT 2022
>> /* An event location used to set a stop event in the inferior.
>> This structure is an amalgam of the various ways
>> to specify where a stop event should be set. */
Andrew> I think this comment is now out of date.
I'll update it.
>> - union
>> + /* Cached string representation of this location. This is used, e.g., to
>> + save stop event locations to file. Malloc'd. */
>> + char *as_string = nullptr;
Andrew> I wonder if you considered making these private, at least for type,
Andrew> and adding a read accessor? For as_string making it protected would
Andrew> seem like a better choice, we still need write access from
Andrew> set_event_location_string, but that itself could become a member
Andrew> function, and get_address_string_location would currently seem to need
Andrew> an accessor, though I notice in a later patch you change
Andrew> get_address_string_location to call to_string.
Andrew> If you think these changes would be better done later then this LGTM.
I didn't really consider it, but yeah, it makes sense.
I'd rather do this as a follow-up, though.
Tom
More information about the Gdb-patches
mailing list