-
-
Notifications
You must be signed in to change notification settings - Fork 188
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
filechooser: Support current_folder with OpenFile #874
Closed
sophie-h
wants to merge
2
commits into
flatpak:main
from
sophie-h:wip/sophie-h/open-file-current-folder
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation would be a lot more useful if it stated what filesystem view will be used to resolve this path - is it a host path ? Or something visible inside the sandbox?
It would also be good to clarify 'suggested' a bit more. I am reading it to mean:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, aren't byte arrays always null-terminated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think D-Bus requires byte arrays to be null-terminated, just as integer arrays aren't required to end with a zero.
Only strings are required to be null-terminated, but a byte array isn't a string as far as D-Bus is concerned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though, I don't know why we want to require null termination of the byte arrays. The null at the end of a string is not considered to be part of the string itself, so similar logic dictates that byte arrays should not semantically include a trailing null byte, if any. (Whether or not the marshalling format will place a null at the end of the byte array is a different matter, but I don't think D-Bus users should rely on that.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that to make this interface useful, the documentation needs to answer this question.
I agree.
D-Bus doesn't require byte arrays to be zero-terminated. A byte array can be any arbitrary blob of bytes, similar to any other integer array but with 1-byte elements; it's like a
struct { unsigned char *location, size_t length }
in C.As a higher-level, application-layer thing, an interface can define specific semantics for a byte array, like "it contains the bytes of a valid JPEG". In this particular case, the interface is using the convention for representing "bytestrings" (conceptually a string in an arbitrary ASCII superset, similar to
(type filename)
in GObject-Introspection), which is to encode them in the D-Bus message as a byte array with lengthstrlen(str) + 1
, containing thestrlen(str)
non-zero bytes of the string, followed by exactly one byte with value zero.In the D-Bus Specification, the LinuxSecurityLabel is currently the only standardized bytestring, and is described as "the non-zero bytes of [whatever] in an unspecified ASCII-compatible encoding, followed by a single zero byte". I think that's a good way to phrase it.
"Null-terminated" is a somewhat misleading term for this, because this is byte value 0 (ASCII mnemonic
NUL
, not aNULL
pointer.It's correct to use the bytestring convention for Unix filenames, environment variable names/values, and any similar string-like thing that (unfortunately) cannot be guaranteed to be UTF-8 (and might instead be a legacy national character set like Latin-1).
The underlying marshalling format does not guarantee that there will be a zero after the end of a byte array, and also does not guarantee that there will not be a zero in the middle of the byte array. The higher-level (application-level) bytestring convention is that there is exactly one zero in the length-counted content of the array, as the last item.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we were optimizing for theoretical correctness over pragmatism, then maybe; but D-Bus is designed to be reasonably efficient and reasonably easy to bind into common programming languages. The bytestring convention is that the length-counted byte array does include the trailing zero, because that means C/C++ bindings can read the bytestring directly out of the message payload without copying, like they can for the (UTF-8) D-Bus string type (for which the marshalling format does guarantee to put a zero immediately after the semantic content of the string).
You can think of it as "it's semantically a byte array containing a serialized data structure that we have chosen, and the serialized data structure we have chosen is a zero-terminated bytestring" if that makes you happer.