GNOME Bugzilla – Bug 752525
A crash caused by malformed array
Last modified: 2021-06-09 16:09:13 UTC
I've prepared these patches when I was debugging https://retrace.fedoraproject.org/faf/reports/683234/. The first problem can be reproduced by setting 'application-children' of 'org.gnome.desktop.notifications' to ['']. I was not able to reproduce the second one but the backtrace says that the pointer 'new_app_ids' has incorrect value so I've initialized it and checked it before use.
Created attachment 307609 [details] [review] Don't crash because of zero-length string
Created attachment 307610 [details] [review] Don't crash because of wrong pointer
Review of attachment 307609 [details] [review]: To avoid what crash? ::: panels/notifications/cc-notifications-panel.c @@ +332,3 @@ GAppInfo *app_info; + if (canonical_app_id[0] == '\0') I prefer: if (*canonical_app_id == '\0')
Review of attachment 307610 [details] [review]: In which case can this happen?
Created attachment 307613 [details] [review] Don't crash because of zero-length string (In reply to Bastien Nocera from comment #3) > Review of attachment 307609 [details] [review] [review]: > > To avoid what crash? Run "gnome-control-center notifications" and then run "gsettings set org.gnome.desktop.notifications application-children "['']"". It will crash the gnome-control-center. > ::: panels/notifications/cc-notifications-panel.c > @@ +332,3 @@ > GAppInfo *app_info; > > + if (canonical_app_id[0] == '\0') > > I prefer: > if (*canonical_app_id == '\0') Done.
(In reply to Bastien Nocera from comment #4) > Review of attachment 307610 [details] [review] [review]: > > In which case can this happen? According to https://retrace.fedoraproject.org/faf/reports/683234/ there is a crash on this line: for (i = 0; new_app_ids[i]; i++) So there is something wrong with the array, most probably the 'new_app_ids' is NULL or not initialized at all.
(In reply to Marek Kašík from comment #5) > Created attachment 307613 [details] [review] [review] > Don't crash because of zero-length string > > (In reply to Bastien Nocera from comment #3) > > Review of attachment 307609 [details] [review] [review] [review]: > > > > To avoid what crash? > > Run "gnome-control-center notifications" and then run "gsettings set > org.gnome.desktop.notifications application-children "['']"". > It will crash the gnome-control-center. Then mention it in the commit message. You also don't explain how you're avoiding the crash, and it's not clear from the patch's context either.
Created attachment 308217 [details] [review] Don't crash because of zero-length string I've updated the commit message.
Review of attachment 308217 [details] [review]: Looks good.
Comment on attachment 308217 [details] [review] Don't crash because of zero-length string Thank you for the review, I've pushed the patch into 3.16 and master.
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org. As part of that, we are mass-closing older open tickets in bugzilla.gnome.org which have not seen updates for a longer time (resources are unfortunately quite limited so not every ticket can get handled). If you can still reproduce the situation described in this ticket in a recent and supported software version, then please follow https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines and create a new bug report at https://gitlab.gnome.org/GNOME/gnome-control-center/-/issues/ Thank you for your understanding and your help.