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