After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 777214 - Crash on enter in search
Crash on enter in search
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: File Search Interface
3.22.x
Other Linux
: Normal normal
: ---
Assigned To: Nautilus Maintainers
Nautilus Maintainers
: 781344 781381 784952 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2017-01-13 13:29 UTC by Robert Roth
Modified: 2017-07-14 14:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
files-view: fix crash on enter in search (1.20 KB, patch)
2017-01-13 21:42 UTC, Alexandru Pandelea
none Details | Review
files-view: fix crash on enter in search (2.05 KB, patch)
2017-01-23 14:52 UTC, Alexandru Pandelea
none Details | Review
files-view: fix crash on enter in search (1.12 KB, patch)
2017-01-24 12:12 UTC, Alexandru Pandelea
none Details | Review
files-view: fix crash on enter in search (1.14 KB, patch)
2017-01-24 12:29 UTC, Alexandru Pandelea
committed Details | Review

Description Robert Roth 2017-01-13 13:29:40 UTC
Steps to reproduce
1. open nautilus
2. type something to search for
3. delete the characters with backspace
4. press enter

Expected: nothing
What happens: Nautilus crashes on pressing enter with the empty search field.
Comment 1 Alexandru Pandelea 2017-01-13 21:42:37 UTC
Created attachment 343456 [details] [review]
files-view: fix crash on enter in search

Pressing enter with no selection while in search makes Nautilus
crash.

The problem is that Nautilus will try to access the first element
of a list that is NULL.

To avoid this, make sure that the list is not NULL.
Comment 2 Ernestas Kulik 2017-01-23 12:31:25 UTC
Review of attachment 343456 [details] [review]:

::: src/nautilus-files-view.c
@@ +1142,3 @@
                                                   NULL);
 
+    if (nautilus_files_view_supports_extract_here (view) && files != NULL)

Sure, but I’m a bit dubious whether the placement of the null check is correct, since the else block will be executed in cases as described here, resulting in an unnecessary function call, etc.

Though I prefer using indentation sparingly, I think the check belongs inside.
Comment 3 Alexandru Pandelea 2017-01-23 14:50:18 UTC
(In reply to Ernestas Kulik from comment #2)
> Review of attachment 343456 [details] [review] [review]:
> 
> ::: src/nautilus-files-view.c
> @@ +1142,3 @@
>                                                    NULL);
>  
> +    if (nautilus_files_view_supports_extract_here (view) && files != NULL)
> 
> Sure, but I’m a bit dubious whether the placement of the null check is
> correct, since the else block will be executed in cases as described here,
> resulting in an unnecessary function call, etc.
> 
> Though I prefer using indentation sparingly, I think the check belongs
> inside.

Thanks for the review. You're right, we shouldn't have that unnecessary function call when the list is NULL
Comment 4 Alexandru Pandelea 2017-01-23 14:52:09 UTC
Created attachment 344038 [details] [review]
files-view: fix crash on enter in search

Pressing enter with no selection while in search makes Nautilus
crash.

The problem is that Nautilus will try to access the first element
of a list that is NULL.

To avoid this, make sure that the list is not NULL.
Comment 5 Ernestas Kulik 2017-01-23 15:44:30 UTC
Review of attachment 344038 [details] [review]:

LGTM, thanks!
Comment 6 Carlos Soriano 2017-01-24 10:56:19 UTC
Review of attachment 344038 [details] [review]:

Hey, thanks Ernestas and Alex!

Sorry to step on your review, but that function doesn't make sense if there is nothing to activate. either we should early return at the start of the function if that's the case or not call this function at all from the callers if this is the case. Probably the former.
Comment 7 Ernestas Kulik 2017-01-24 11:30:45 UTC
(In reply to Carlos Soriano from comment #6)
> Sorry to step on your review, but that function doesn't make sense if there
> is nothing to activate. either we should early return at the start of the
> function if that's the case or not call this function at all from the
> callers if this is the case. Probably the former.

Yeah, that makes more sense.
Comment 8 Alexandru Pandelea 2017-01-24 12:12:44 UTC
Created attachment 344139 [details] [review]
files-view: fix crash on enter in search

Pressing enter with no selection while in search makes Nautilus
crash.

The problem is that Nautilus will try to access the first element
of a list that is NULL.

To avoid this, make sure that the list is not NULL.
Comment 9 Carlos Soriano 2017-01-24 12:21:58 UTC
Review of attachment 344139 [details] [review]:

Just push after fixing it

::: src/nautilus-files-view.c
@@ +1139,3 @@
+    if (files == NULL)
+        return;
+

The new code style!!! :D

if ()
{
    return;
}
Comment 10 Alexandru Pandelea 2017-01-24 12:29:03 UTC
(In reply to Carlos Soriano from comment #9)
> Review of attachment 344139 [details] [review] [review]:
> 
> Just push after fixing it
> 
> ::: src/nautilus-files-view.c
> @@ +1139,3 @@
> +    if (files == NULL)
> +        return;
> +
> 
> The new code style!!! :D
> 
> if ()
> {
>     return;
> }

Ah, that's right, sry :D
Comment 11 Alexandru Pandelea 2017-01-24 12:29:31 UTC
Created attachment 344146 [details] [review]
files-view: fix crash on enter in search

Pressing enter with no selection while in search makes Nautilus
crash.

The problem is that Nautilus will try to access the first element
of a list that is NULL.

To avoid this, make sure that the list is not NULL.
Comment 12 Alexandru Pandelea 2017-01-24 12:41:59 UTC
Attachment 344146 [details] pushed as 82a2d53 - files-view: fix crash on enter in search
Comment 13 Ernestas Kulik 2017-04-16 06:00:58 UTC
*** Bug 781344 has been marked as a duplicate of this bug. ***
Comment 14 Ernestas Kulik 2017-04-16 18:57:18 UTC
*** Bug 781381 has been marked as a duplicate of this bug. ***
Comment 15 Ernestas Kulik 2017-07-14 14:14:25 UTC
*** Bug 784952 has been marked as a duplicate of this bug. ***