mib accepted this revision.
mib added a comment.
This revision is now accepted and ready to land.
LGTM!
================
Comment at: lldb/examples/python/crashlog.py:515
def parse_images(self, json_images):
- idx = 0
- for json_image in json_images:
+ for idx, json_image in enumerate(json_images):
img_uuid = uuid.UUID(json_image['uuid'])
----------------
JDevlieghere wrote:
> mib wrote:
> > mib wrote:
> > > JDevlieghere wrote:
> > > > mib wrote:
> > > > > What do we use `idx` for ?
> > > > You're right, this isn't necessary anymore.
> > > I'm really not a big fan of having very similar image lists ... may be we
> > > could use the from the crashlog object and skip the first entry (since we
> > > know it's the main executable).
> > > What do you think ?
> > Otherwise, we could hoist the main executable image from the image list and
> > handle it separately.
> I understand the concern. To be fair, I didn't check whether the main
> executable coming first is something we rely on, but I'm pretty sure we are:
> we'll need it to create the target. I didn't want to mess with that and risk
> introducing a bug that way. It took me quite some time to figure out this was
> an issue when parsing the symbol data. If we don't want to break that
> assumption, there's nothing more efficient than keeping a second list of
> references. I also think it makes sense to keep that in the JSON parser,
> because the index of (parsed) image is only something that makes sense for
> that format because it cross references images based on their index. That's
> not the case in the textual or parser crashlogs.
>
> FWIW this is the code that moves the main image to the top, invalidating the
> image indexes of every image before it:
>
> ```
> def set_main_image(self, identifier):
> for i, image in enumerate(self.images):
> if image.identifier == identifier:
> self.images.insert(0, self.images.pop(i))
> break
> ```
>
Fair
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148172/new/
https://reviews.llvm.org/D148172
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
