Use sealded memfd for icon validator #1424
Conversation
Since we are the only user of the icon-validator we don't need to split the hard coded limits for icon validation between xdp-utils and the icon-validator itself. This moves icon size limit to the icon-validator and drops the second format check from xdp-validate_serialized_icon.
|
@GeorgesStavracas last time we talked about this I think we could agree on making the validator take an argument which specifies what kind of icon should be validated and have all the logic about what a valid icon kind looks like in the validator itself. Do I remember that right? |
Yes. But I'm tending not to block this PR on it, it can be a follow-up cleanup just fine. |
jsparber
force-pushed
the
rework_image_validator
branch
from
0087624 to
18192c4
Compare
|
A few more nit picks about alignment and use of gchar/gint and documentation but when they are dealt with this is good to land IMO. |
jsparber
force-pushed
the
rework_image_validator
branch
2 times, most recently
from
c6a60f0 to
602d4e1
Compare
There aren't many places where we need to spawn a subprocess but in all cases we have a list of commend args and need the output as a response. Make `xdp_spawn()` and `xdp_spawnv()` nicer to use and only take arguments that are actually used.
This allows to use the same spawn methods to spawn the icon validator, in a future commit it switch to this method and start using fds instead of temp files.
jsparber
force-pushed
the
rework_image_validator
branch
from
602d4e1 to
4719bb6
Compare
jsparber
force-pushed
the
rework_image_validator
branch
from
4719bb6 to
77f3ef1
Compare
There was a problem hiding this comment.
My comments are mostly superficial at this point, my big complaint is that I think inline assignments are overused in these patches. My rule of thumb for this is, only assign variables in ifs / fors / whiles lines when the variable is only going to be used inside the block (i.e. it's an scoped variable).
This reduces the amount of data we need to copy once we allow using sealable memfd for passing icons, and we can also store bytes icons in a memfd instead of a temp file.
Images shouldn't be to big in size. 4MBs is more then enough for all cases.
This object is used to to store GBytes that need to be passed between processes. In future it will also be used to share data between clients, the frontend and backend. The introduced object handles creating and sealing of memfds. It uses memfd_create() to create a new fd in case GBytes are provided and seals it. It also takes already existing memfds and ensures all seals that prevent data modifications are applied.
This will allow us in future to write GBytesIcons once to a shareable read-only fd. So that we don't need to load the same date in multiple places. Also in a future commit the notification portal will start to accept sealable memfds (stored in XdpSealedFd) that will be passed onto the backend. In general, there are multiple places where we can consume an fd instead of using serialized data (including sending over DBus).
jsparber
force-pushed
the
rework_image_validator
branch
from
77f3ef1 to
1352c45
Compare
I split out the first few commits from #1298 that rework how the validator works and that introduce the
XdpSealdFd.CC: @hfiguiere and @GeorgesStavracas