On Sat, Mar 28, 2015 at 09:40:05PM +0100, Richard Weinberger wrote:
> > diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
> > index 1249f91..c955976 100644
> > --- a/drivers/staging/speakup/main.c
> > +++ b/drivers/staging/speakup/main.c
> > @@ -423,7 +423,8 @@ static void announce_edge(struct vc_data *vc, int msg_id)
> > if (spk_bleeps & 1)
> > bleep(spk_y);
> > if ((spk_bleeps & 2) && (msg_id < edge_quiet))
> > - synth_printf("%s\n", spk_msg_get(MSG_EDGE_MSGS_START + msg_id - 1));
> > + synth_printf("%s\n",
> > + spk_msg_get(MSG_EDGE_MSGS_START + msg_id - 1));
>
> Instead of blindly adding newlines to silence checkpatch.pl, what
> about reworking the code?
> printf("%s\n", ..) cries for a puts().
printf is fine. Not everything has to be a massive engineering project
which supports every method in the world.
>
> > }
> >
> > static void speak_char(u_char ch)
> > @@ -1131,7 +1132,8 @@ static void spkup_write(const char *in_buf, int count)
> > if (in_count > 2 && rep_count > 2) {
> > if (last_type & CH_RPT) {
> > synth_printf(" ");
> > - synth_printf(spk_msg_get(MSG_REPEAT_DESC2), ++rep_count);
> > + synth_printf(spk_msg_get(MSG_REPEAT_DESC2),
> > + ++rep_count);
> > synth_printf(" ");
>
> This printf stuff looks odd. synth_printf() seems to take a format
> string, in this case the format string
> is returned by spk_msg_get(), smells like a format string bug.
It's not a bug, but it's definitely odd. I think the reason they did it
that way is so they can translate the output to other languages.
Anyway, this patch is basically fine.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
