GNOME Bugzilla – Bug 773150
Segfault on pressing Enter/activate in list view with nothing selected
Last modified: 2016-10-30 14:46:19 UTC
Open Nautilus in list view, then press the right arrow to move focus from the sidebar to the list view, but without selecting any items. Now press Enter. Nautilus tries to activate the selected items, but there are none, so it bails with a segfault. Backtrace: Thread 1 "nautilus" received signal SIGSEGV, Segmentation fault. 0x0000000000490e43 in nautilus_files_view_activate_files (view=0xa571c0, files=files@entry=0x0, flags=flags@entry=(unknown: 0), confirm_multiple=confirm_multiple@entry=1) at nautilus-files-view.c:1176 1176 location = nautilus_file_get_location (NAUTILUS_FILE (g_list_first (files)->data)); (gdb) bt
+ Trace 236750
Created attachment 337934 [details] [review] list-view: do not try to activate a NULL selection Only try to activate if the list of selected items is non-NULL.
Review of attachment 337934 [details] [review]: Nice catch, thanks. Time for me to pick some nits, now. :) It would be great if you could write something in the body of the commit message as well. A short description of the issue will do. ::: src/nautilus-list-view.c @@ +230,2 @@ file_list = nautilus_list_view_get_selection (NAUTILUS_FILES_VIEW (view)); + if (file_list) For the sake of being explicit, change this to “file_list != NULL”. IIRC, it is all over the place right now, but I try to do this more often.
Thanks, will do. Before that - in cases like this, is it OK to combine a variable's declaration and assignment on the same line? I see it done sometimes, but not usually. Is there a particular rule for when this can be done?
Created attachment 337940 [details] [review] list-view: do not try to activate a NULL selection > It is possible to give focus to the list view with nothing selected. On > pressing Enter, Nautilus would try to activate a null list of items and > therefore segfault. Fix this by doing nothing if there is no selection. This also adds the explicit comparison to NULL as requested by Ernestas. (Following my previous comment, I'm assuming you want the variable declaration kept separate, as it originally was, and like most other code in the project.)
(In reply to djb from comment #3) > Thanks, will do. > > Before that - in cases like this, is it OK to combine a variable's > declaration and assignment on the same line? I see it done sometimes, but > not usually. Is there a particular rule for when this can be done? Hard to say, really. I do not think we have reached a consensus there. For one, it helps keep lines short and sweet (we have a soft limit of 80 characters and a hard one of 100), but it also inflates the line count. It is best if you follow the style of existing code, but I will not bat an eyelid if you choose one over the other in your code, personally.
(In reply to Ernestas Kulik from comment #5) > it helps keep lines short and sweet Err, not doing so, I mean.
Review of attachment 337940 [details] [review]: Sure, looks fine to me!
Attachment 337940 [details] pushed as 9fa6a1f - list-view: do not try to activate a NULL selection
*** Bug 773699 has been marked as a duplicate of this bug. ***