Skip to content
This repository has been archived by the owner on Feb 12, 2023. It is now read-only.

fix(chatform): Fix invalid access of empty container #5848

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sphaerophoria
Copy link
Contributor

@sphaerophoria sphaerophoria commented Sep 21, 2019

This partially fixes #5832 in that we no long segfault. There is a
deeper underlying problem that the history still relies on undefined
behavior of GROUP BY causing us to get an empty container in some cases.


This change is Reviewable

Copy link
Member

@sudden6 sudden6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 1 LGTMs obtained


src/widget/form/genericchatform.cpp, line 689 at r1 (raw file):

    auto end = ChatLogIdx(0);
    if (time.isNull()) {
        end = messages.begin()->first;

Isn't this also a potential crash if messages is empty?


src/widget/form/genericchatform.cpp, line 720 at r1 (raw file):

    auto begin = ChatLogIdx(0);
    if (time.isNull()) {
        begin = messages.rbegin()->first;

same

@anthonybilinski
Copy link
Member

rebased to v1.17-dev tip. It does look like there are a couple unguarded usages of messages's first item.

This partially fixes qTox#5832 in that we no long segfault. There is a
deeper underlying problem that the history still relies on undefined
behavior of GROUP BY causing us to get an empty container in some cases.
@anthonybilinski anthonybilinski changed the base branch from v1.17-dev to master February 1, 2021 13:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants