From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751955AbbC1VXE (ORCPT ); Sat, 28 Mar 2015 17:23:04 -0400 Received: from a.ns.miles-group.at ([95.130.255.143]:65275 "EHLO radon.swed.at" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751232AbbC1VXB (ORCPT ); Sat, 28 Mar 2015 17:23:01 -0400 Message-ID: <55171BAF.9050405@nod.at> Date: Sat, 28 Mar 2015 22:22:55 +0100 From: Richard Weinberger User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Joe Perches CC: Shirish Gajera , w.d.hubbs@gmail.com, chris@the-brannons.com, kirk@reisers.ca, samuel.thibault@ens-lyon.org, Greg KH , =?UTF-8?B?RG9tYWdvaiBUcsWhYW4=?= , mahfouz.saif.elyazal@gmail.com, Ben Hutchings , roxanagabriela10@gmail.com, Robin Schroer , dilekuzulmez@gmail.com, DaeSeok Youn , =?UTF-8?B?QXnFn2UgTWVsaWtlIFl1cnRvxJ9sdQ==?= , Rusty Russell , tapaswenipathak@gmail.com, vthakkar1994@gmail.com, speakup@linux-speakup.org, "devel@driverdev.osuosl.org" , LKML Subject: Re: [PATCH] staging: speakup: Fix warning of line over 80 characters. References: <20150328202139.GA12695@shirish-ThinkPad-Edge-E430> <1427577518.2715.8.camel@perches.com> In-Reply-To: <1427577518.2715.8.camel@perches.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am 28.03.2015 um 22:18 schrieb Joe Perches: > On Sat, 2015-03-28 at 21:40 +0100, Richard Weinberger wrote: >> On Sat, Mar 28, 2015 at 9:21 PM, Shirish Gajera wrote: >>> This patch fixes the checkpatch.pl warning: > > [] > >>> diff --git 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(). > > There is no synth_puts So what? Fix it! :-) >>> @@ -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. > > Nope, but it would be nicer to avoid these spk_msg_get > functions for the indices that are used with printf style > formatting. > >>> } >>> rep_count = 0; >>> @@ -1847,7 +1849,8 @@ static void speakup_win_set(struct vc_data *vc) >>> win_right = spk_x; >>> } >>> snprintf(info, sizeof(info), spk_msg_get(MSG_WINDOW_BOUNDARY), >>> - (win_start) ? spk_msg_get(MSG_END) : spk_msg_get(MSG_START), >>> + (win_start) ? >>> + spk_msg_get(MSG_END) : spk_msg_get(MSG_START), >>> (int)spk_y + 1, (int)spk_x + 1); >> >> Same here. Also please resolve the ?: mess. > > I don't think there's a ?: mess, but the code looks wrong. > > win_start ? MSG_END : MSG_START Face it, the whole code is horrible and lines other 80 chars are the *least* problem. Submitting a patch just for the sake of silencing checkpatch.pl is a waste of time. After applying this patch the driver 0 better than before. Thanks, //richard