On Tue, 2021-03-09 at 12:43 +0100, Michal Privoznik wrote:
On 3/5/21 8:13 PM, Andrea Bolognani wrote:
> + if (!g_file_get_contents(procfile, &buf, &len, NULL))
> + return -1;
I did not spot this yesterday, but now I'm working on a something else
and have to read a contents of a file under /proc. I did not recall the
exact name but remembered where I saw it lately - here :-)
And now that I am thinking about it - and reading the docs - is this
function safe? I mean, it reads file without any limit - which may be
fine for /proc files, but I worry that if allowed in one func it may
sneak into others and read user provided files, or while its use in a
function X might be warranted for now, in the future after some refactor
the function X might be used to read user provided files.
You're right. I used pure GLib functions initially because I was
implementing this as a tiny stand-alone tool for faster iterative
development, and I just forgot to change that specific function back
to the libvirt equivalent when I was done :)
Therefore, I think it should go onto the list of not-on-my-watch
functions and we ought stick with our fine crafted virFileRead*().
BTW: I think the same about g_get_host_name(), which does not reflect
hostname changes. Unfortunately, we have three places which slipped
through while I wasn't watching. I'll look into how to revert them.
Sounds good.
--
Andrea Bolognani / Red Hat / Virtualization