Hi,
On Mon, Jul 15, 2019 at 05:13:43AM -0400, Frediano Ziglio wrote:
> >
> > From: Victor Toso <me@xxxxxxxxxxxxxx>
> >
> > | Error: CHECKED_RETURN (CWE-252):
> > | spice-vdagent-0.19.0/src/vdagentd/vdagentd.c:999: check_return: Calling
> > | "fprintf" without checking return value (as is done elsewhere 29 out of
> > | 30 times).
> > | spice-vdagent-0.19.0/src/vdagentd/xorg-conf.c:95: example_assign: Example
> > | 1: Assigning: "r" = return value from "fprintf(f, "# xorg.conf generated
> > | by spice-vdagentd\n")".
> > | spice-vdagent-0.19.0/src/vdagentd/xorg-conf.c:95: example_checked: Example
> > | 1 (cont.): "r" has its value checked in "r < 0".
> > | spice-vdagent-0.19.0/src/vdagentd/xorg-conf.c:96: example_assign: Example
> > | 2: Assigning: "r" = return value from "fprintf(f, "# generated from
> > | monitor info provided by the client\n\n")".
> > | spice-vdagent-0.19.0/src/vdagentd/xorg-conf.c:96: example_checked: Example
> > | 2 (cont.): "r" has its value checked in "r < 0".
> > | spice-vdagent-0.19.0/src/vdagentd/xorg-conf.c:99: example_assign: Example
> > | 3: Assigning: "r" = return value from "fprintf(f, "# Client has only 1
> > | monitor\n")".
> > | spice-vdagent-0.19.0/src/vdagentd/xorg-conf.c:99: example_checked: Example
> > | 3 (cont.): "r" has its value checked in "r < 0".
> > | spice-vdagent-0.19.0/src/vdagentd/xorg-conf.c:100: example_assign: Example
> > | 4: Assigning: "r" = return value from "fprintf(f, "# This works best with
> > | no xorg.conf, leaving xorg.conf empty\n")".
> > | spice-vdagent-0.19.0/src/vdagentd/xorg-conf.c:100: example_checked:
> > | Example 4 (cont.): "r" has its value checked in "r < 0".
> > | spice-vdagent-0.19.0/src/vdagentd/xorg-conf.c:106: example_assign: Example
> > | 5: Assigning: "r" = return value from "fprintf(f, "Section
> > | \"ServerFlags\"\n")".
> > | spice-vdagent-0.19.0/src/vdagentd/xorg-conf.c:106: example_checked:
> > | Example 5 (cont.): "r" has its value checked in "r < 0".
> > | # 997| pidfile = fopen(pidfilename, "w");
> > | # 998| if (pidfile) {
> > | # 999|-> fprintf(pidfile, "%d\n", (int)getpid());
> > | # 1000| fclose(pidfile);
> > | # 1001| }
> >
> > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> > ---
> > src/vdagentd/vdagentd.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
> > index 72a3e13..63f3a12 100644
> > --- a/src/vdagentd/vdagentd.c
> > +++ b/src/vdagentd/vdagentd.c
> > @@ -996,7 +996,11 @@ static void daemonize(void)
> > }
> > pidfile = fopen(pidfilename, "w");
> > if (pidfile) {
> > - fprintf(pidfile, "%d\n", (int)getpid());
> > + int pid = (int) getpid();
> > + int r = fprintf(pidfile, "%d\n", pid);
> > + if (r < 0) {
> > + syslog(LOG_ERR, "Failure to write pid %d to file %s", pid,
> > pidfilename);
> > + }
> > fclose(pidfile);
> > }
> > break;
>
> I would say it's pretty weird to give error if a rare situation
> happens (unable to write small data) but if we cannot create
> the file (which is much more common) there's no warning/error.
Indeed, I guess I was a bit too focused on cleaning the list of
warnings and didn't pay that much attention. I'll rework this.
> Why you had to add an additional "pid" variable?
I usually cache the value in a local var instead of calling the
function more than once. Not a hard rule but I usually like the
code more.
Thanks,
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel