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 659212 - GMappedFile should fail on non-regular files
GMappedFile should fail on non-regular files
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2011-09-16 02:52 UTC by Behdad Esfahbod
Modified: 2011-09-18 00:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GMappedFile: fail when mapping a device file (2.08 KB, patch)
2011-09-16 02:54 UTC, Allison Karlitskaya (desrt)
accepted-commit_now Details | Review
GMappedFile: return an error when trying to map a device (3.56 KB, patch)
2011-09-18 00:03 UTC, Matthias Clasen
committed Details | Review
GMappedFile: return an error when trying to map a device (3.36 KB, patch)
2011-09-18 00:03 UTC, Matthias Clasen
committed Details | Review
GMappedFile: fail when mapping a device file (2.08 KB, patch)
2011-09-18 00:03 UTC, Matthias Clasen
committed Details | Review

Description Behdad Esfahbod 2011-09-16 02:52:04 UTC
Currently it returns an empty file because fstat() returns 0 filesize and we return early.  Correct fix should return an error.  Ideally an error that can be detected such that the caller can revert back to fread()ing.

While discussing this, we figured that the documentation for G_FILE_ERROR_NODEV is also inadequate.
Comment 1 Allison Karlitskaya (desrt) 2011-09-16 02:54:11 UTC
Created attachment 196687 [details] [review]
GMappedFile: fail when mapping a device file

mmap() fails on zero-sized files, so we previously had a special case to
avoid calling it in that case.  Unfortunately, this had the side effect
of causing us to fail to notice that we were attempting to mmap() a
device node.

Modify the special-casing to only apply in the case that we're dealing
with a normal file.
Comment 2 Matthias Clasen 2011-09-16 13:49:46 UTC
Review of attachment 196687 [details] [review]:

Looks good to me
Comment 3 Behdad Esfahbod 2011-09-16 16:41:37 UTC
I still appreciate a documented error code that lets the caller decide to try g_file_get_contents() instead.
Comment 4 Matthias Clasen 2011-09-18 00:03:27 UTC
The following fixes have been pushed:
f18eab2 GMappedFile: return an error when trying to map a device
500202a GMappedFile: return an error when trying to map a device
817466f GMappedFile: fail when mapping a device file
Comment 5 Matthias Clasen 2011-09-18 00:03:31 UTC
Created attachment 196853 [details] [review]
GMappedFile: return an error when trying to map a device

Previously, we were returning an empty buffer for all filenames
where fstat() gives a size of 0. But this is only appropriate
for regular files.

Also improve the documentation around this issue. Based on a
patch by Ryan Lortie.

Conflicts:

	glib/tests/mappedfile.c
Comment 6 Matthias Clasen 2011-09-18 00:03:34 UTC
Created attachment 196854 [details] [review]
GMappedFile: return an error when trying to map a device

Previously, we were returning an empty buffer for all filenames
where fstat() gives a size of 0. But this is only appropriate
for regular files.

Also improve the documentation around this issue. Based on a
patch by Ryan Lortie.
Comment 7 Matthias Clasen 2011-09-18 00:03:37 UTC
Created attachment 196855 [details] [review]
GMappedFile: fail when mapping a device file

mmap() fails on zero-sized files, so we previously had a special case to
avoid calling it in that case.  Unfortunately, this had the side effect
of causing us to fail to notice that we were attempting to mmap() a
device node.

Modify the special-casing to only apply in the case that we're dealing
with a normal file.