Summary: | [Seccomp Filters] Add helpers to get XDG base directory locations | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> | ||||||||||
Component: | WebKit2 | Assignee: | Michael Catanzaro <mcatanzaro> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Enhancement | CC: | mcatanzaro, zan | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | Linux | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 140072, 142987 | ||||||||||||
Attachments: |
|
Description
Michael Catanzaro
2015-03-23 14:02:59 PDT
Created attachment 249268 [details]
[Linux] Seccomp Filters: Add helper functions for common directories
Created attachment 256704 [details]
[Linux] Seccomp Filters: Add helper functions for common directories
Comment on attachment 256704 [details]
[Linux] Seccomp Filters: Add helper functions for common directories
This one's next, but I'm pretty sure I should find a better home for these files than WebKit2/Shared/linux/SeccompFilters. Maybe WebCore/platform/FileSystemGLib.cpp?
The point of these functions, btw, is to use the GLib base directory functions without actually making SyscallPolicy.cpp depend on GLib. (In reply to comment #4) > The point of these functions, btw, is to use the GLib base directory > functions without actually making SyscallPolicy.cpp depend on GLib. Which maybe is not a worthwhile venture, since GTK, EFL, and WPE all use GLib. I think it's okay to abstract away the GLib functions, but I'm not sure where to put the files. One option could be Source/WebCore/platform/seccompfilters, or a similarly named directory. The addition is specific to this feature, and the GLib-specific implementation could go into a UserDirectoriesGLib.cpp or similar, along with the UserDirectories.h header. ('BaseDirectory' sound odd to me.) FileSystem.h is a more generic API used throughout the code, but the additions would for the moment only be used in seccomp filters and nowhere else, so half of the ports would end up with unused stub implementations, or the additions would be #if-guarded away (which I dislike). Dunno. (In reply to comment #6) > I think it's okay to abstract away the GLib functions, but I'm not sure > where to put the files. > > One option could be Source/WebCore/platform/seccompfilters, or a similarly > named directory. I'd rather keep all the seccompfilters stuff in the WebKit2 layer. So that suggests we should keep it exactly where I added it in this patch, except perhaps renaming the file: > The addition is specific to this feature, and the > GLib-specific implementation could go into a UserDirectoriesGLib.cpp or > similar, along with the UserDirectories.h header. ('BaseDirectory' sound odd > to me.) UserDirectories is fine by me. I picked BaseDirectory because the functions are wrappers for the XDG Base Directory Specification [1], but I don't care what we call the file. [1] http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html XDGBaseDirectory.h could fit then as well. Wasn't aware of the spec. Comment on attachment 256704 [details] [Linux] Seccomp Filters: Add helper functions for common directories View in context: https://bugs.webkit.org/attachment.cgi?id=256704&action=review The remaining thing I'd recommend is to rename the implementation file into BaseDirectoryGLib.cpp (or whatever name we end up choosing), since the implementation depends on and is specific to GLib-using ports. > Source/WebKit2/Shared/linux/SeccompFilters/BaseDirectory.cpp:32 > +#include <glib.h> > + > +#if ENABLE(SECCOMP_FILTERS) glib.h should be included after the ENABLE() guard. Created attachment 257270 [details]
Patch
Created attachment 257271 [details]
Patch
(In reply to comment #9) > The remaining thing I'd recommend is to rename the implementation file into > BaseDirectoryGLib.cpp (or whatever name we end up choosing), since the > implementation depends on and is specific to GLib-using ports. OK, done. I went with XDGBaseDirectoryGLib.cpp. > glib.h should be included after the ENABLE() guard. Fixed. Committed r187159: <http://trac.webkit.org/changeset/187159> |