hi, comment in-line.
> Hi!
> > +void set_sys_tune(char *sys_file, long tune)
> > +{
> > + int fd;
> > + char buf[BUFSIZ];
> > +
> > + fd = open(sys_file, O_WRONLY);
> > + if (fd == -1)
> > + tst_brkm(TBROK|TERRNO, cleanup, "open");
> > + sprintf(buf, "%ld", tune);
> > + if (write(fd, buf, strlen(buf)) == -1)
> > + tst_brkm(TBROK|TERRNO, cleanup, "write");
> > + close(fd);
> > +}
>
> I would use snprintf() here, even if you are sure the result fits into
> the buffer.
>
> Also adding name of the file to the tst_brkm() report would be better.
>
> So you would get:
>
> open 'filename': no such file or directory
>
> rather than:
>
> open : no such file or directory
have updated it, please check the next email.
>
>
> > +long get_sys_tune(char *sys_file)
> > +{
> > + int fd;
> > + long tune;
> > + char buf[BUFSIZ], *endptr;
> > +
> > + fd = open(sys_file, O_RDONLY);
> > + if (fd == -1)
> > + tst_brkm(TBROK|TERRNO, cleanup, "open");
> > + if (read(fd, buf, BUFSIZ) < 0)
> > + tst_brkm(TBROK|TERRNO, cleanup, "read");
>
> Here as well.
>
> > + close(fd);
> > +
> > + tune = strtol(buf, &endptr, 10);
> > + if (((tune == LONG_MAX || tune == LONG_MIN) && errno == ERANGE) ||
> > + (errno != 0 && tune == 0))
> > + tst_brkm(TBROK|TERRNO, cleanup, "strtol");
>
> I think you should clear errno before calling the strtol, otherwise
> you
> could get invalid conversion for string "0" when errno was set by some
> previous system call.
>
I removed the errno in updated patch.
> > + if (endptr == buf || (*endptr != '\0' && *endptr != '\n'))
> > + tst_brkm(TBROK, cleanup, "Invalid number parameter: %s", buf);
> > +
> > + return tune;
> > +}
> >
> > +/*
> > + * the function is designed to make sure the value we get from
> > + * sys_file is equal to what we set last.
> > + */
> > +void check_sys_tune(char *sys_file, long tune)
> > +{
> > + long val;
> > +
> > + val = get_sys_tune(sys_file);
> > + if (val == tune)
> > + tst_resm(TINFO, "val in %s = %ld", sys_file, tune);
> > + else
> > + tst_brkm(TBROK, cleanup, "val in %s = %ld, not %ld",
> > + sys_file, val, tune);
> > +}
> > +
> > +unsigned long read_meminfo(char *item)
> > +{
> > + FILE *fp;
> > + char line[BUFSIZ], buf[BUFSIZ];
> > + unsigned long val;
> > +
> > + fp = fopen(PATH_MEMINFO, "r");
> > + if (fp == NULL)
> > + tst_brkm(TBROK|TERRNO, cleanup, "fopen %s", PATH_MEMINFO);
> > + while (fgets(line, BUFSIZ, fp) != NULL) {
> > + if (sscanf(line, "%s %lu ", buf, &val) == 2)
>
> Using sscanf() to fill string buffer is potentialy dangerous. You
> could
> either to use GNU extension to hardcode maximal buffer length into the
> format string or to go trough the string while you find whitespace
> (see
> isspace()).
I'm not sure how to changed it, could you give me more details about how to
avoid
Using sscanf(). and always I think Using sscanf() can solve this question.
>
> > + if (strcmp(buf, item) == 0) {
> > + fclose(fp);
> > + return val;
> > + }
> > + continue;
> > + }
> > + fclose(fp);
> > +
> > + tst_brkm(TBROK, cleanup, "cannot find \"%s\" in %s",
> > + item, PATH_MEMINFO);
> > +}
>
>
> --
> Cyril Hrubis
> [email protected]
--
Thanks,
Zhouping Liu
------------------------------------------------------------------------------
Why Cloud-Based Security and Archiving Make Sense
Osterman Research conducted this study that outlines how and why cloud
computing security and archiving is rapidly being adopted across the IT
space for its ease of implementation, lower cost, and increased
reliability. Learn more. http://www.accelacomm.com/jaw/sfnl/114/51425301/
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
