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
