On Sat, May 16, 2020 at 11:53:58PM +0530, Lokesh Vutla wrote:
> diff --git a/makefile b/makefile
> index a965bd4..094c062 100644
> --- a/makefile
> +++ b/makefile
> @@ -22,7 +22,7 @@ CC = $(CROSS_COMPILE)gcc
> VER = -DVER=$(version)
> CFLAGS = -Wall $(VER) $(incdefs) $(DEBUG) $(EXTRA_CFLAGS)
> LDLIBS = -lm -lrt $(EXTRA_LDFLAGS)
> -PRG = ptp4l hwstamp_ctl nsm phc2sys phc_ctl pmc timemaster
> +PRG = ptp4l hwstamp_ctl nsm phc2sys phc_ctl pmc timemaster phc2pwm
The makefile just changed on master. Please re-base.
> +static void usage(char *progname)
> +{
> + fprintf(stderr,
> + "\n"
> + "usage: %s [options]\n\n"
> + "\n"
> + " -c [id] PWM channel id from PWM chip\n"
> + " -e [id] PTP index for event/trigger\n"
> + " -h prints this message and exits\n"
> + " -l [num] set the logging level to 'num'\n"
Could we please have the -m and -q flags, just like the other programs?
> + " -p [dev] Clock device to use\n"
> + " -v prints the software version and exits\n"
> + " -w [id] PWM chip device id\n"
> + "\n",
> + progname);
> +}
> +static uint64_t pwm_servo_sample(struct pwm_servo *ps, uint64_t ts)
> +{
> + double avg_period, ts_diff;
> + int offset, i, drift_cnt;
> + uint64_t period, base;
Please initialize 'period' here, or provide a default: case that
returns zero.
> +int main(int argc, char *argv[])
> +{
> + unsigned int pwm_chip = 0, pwm_chan = 0, event_index = 0;
> + int c, err, level = LOG_INFO;
> + char *progname, *ptp_dev;
Here ptp_dev has a random, likely non-zero value from the stack...
> + struct pwm_chan *chan;
> + struct pwm_servo ps;
> + clockid_t clkid;
> + uint64_t ts;
> +
> + handle_term_signals();
> +
> + /* Process the command line arguments. */
> + progname = strrchr(argv[0], '/');
> + progname = progname ? 1+progname : argv[0];
> +
> + while (EOF != (c = getopt(argc, argv, "c:e:hl:p:vw:"))) {
> + switch (c) {
> + case 'c':
> + pwm_chan = atoi(optarg);
> + break;
> + case 'e':
> + event_index = atoi(optarg);
> + break;
> + case 'h':
> + usage(progname);
> + return 0;
> + case 'l':
> + level = atoi(optarg);
> + break;
> + case 'p':
> + ptp_dev = optarg;
... if the use leaves off the -p flag, then ...
> + break;
> + case 'v':
> + version_show(stdout);
> + return 0;
> + case 'w':
> + pwm_chip = atoi(optarg);
> + break;
> + case '?':
> + default:
> + usage(progname);
> + return -1;
> + }
> + }
> +
> + if (!ptp_dev) {
... this check still fails, and ...
> + usage(progname);
> + return -1;
> + }
> +
> + handle_term_signals();
> + print_set_progname(progname);
> + print_set_level(level);
> +
> + clkid = phc_open(ptp_dev);
now you pass a random pointer value into phc_open(). So, this time
the compiler warning about ptp_dev being uninitialized does identify a
real bug!
Thanks,
Richard
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
