From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756799AbbE2SUu (ORCPT ); Fri, 29 May 2015 14:20:50 -0400 Received: from mail-bl2on0143.outbound.protection.outlook.com ([65.55.169.143]:34497 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752628AbbE2SUk (ORCPT ); Fri, 29 May 2015 14:20:40 -0400 Authentication-Results: spf=none (sender IP is 165.204.84.222) smtp.mailfrom=amd.com; alien8.de; dkim=none (message not signed) header.d=none; X-WSS-ID: 0NP4IY7-08-3O7-02 X-M-MSG: Message-ID: <5568ADEF.6080907@amd.com> Date: Fri, 29 May 2015 13:20:31 -0500 From: Aravind Gopalakrishnan User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Borislav Petkov CC: , , , Subject: Re: [PATCH 3/6] edac, mce_amd_inj: Modify flags attrigute to use string arguments References: <1432753418-2985-1-git-send-email-Aravind.Gopalakrishnan@amd.com> <1432753418-2985-4-git-send-email-Aravind.Gopalakrishnan@amd.com> <20150529134943.GF31435@pd.tnic> In-Reply-To: <20150529134943.GF31435@pd.tnic> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.180.168.240] X-EOPAttributedMessage: 0 X-Microsoft-Exchange-Diagnostics: 1;BN1BFFO11OLC003;1:epELsvhtN/FbLzkgX1IsAyjqGlP9pF0DleUkjBFP48m3KF1seVt9nRmF1TsnmXPPVAycIfMHyAW2lcBmtk4TzKfP3DRlFl6N8JSaaXet22/pzpBvXD8YSO2kN6NkSLEMOIl45ZB4Ld7JTvrG8AEk5jpQF1g0WclW9uAmoNLdUCBCsdd1Xvn/xwEGZBIwJ1y85LXvtcOu3olIYGttC3SH0DmSNrjMwL/ZE9m3qJVbcrHdv/VRIIDwcvCHrNDlL1TWMzEyNX/25Btq51THqCu3tw== X-Forefront-Antispam-Report: CIP:165.204.84.222;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10019020)(979002)(6009001)(428002)(164054003)(377454003)(51704005)(189002)(199003)(24454002)(479174004)(92566002)(64126003)(54356999)(36756003)(64706001)(101416001)(83506001)(23676002)(86362001)(87936001)(5001830100001)(105586002)(65816999)(106466001)(77156002)(5001860100001)(59896002)(68736005)(62966003)(77096005)(80316001)(120886001)(110136002)(33656002)(4001350100001)(76176999)(65806001)(87266999)(189998001)(50986999)(97736004)(50466002)(2950100001)(47776003)(4001540100001)(65956001)(46102003)(969003)(989001)(999001)(1009001)(1019001);DIR:OUT;SFP:1102;SCL:1;SRVR:BN1PR02MB072;H:atltwp02.amd.com;FPR:;SPF:None;PTR:InfoDomainNonexistent;MX:1;A:1;LANG:en; X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BN1PR02MB072; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(520003)(5005006)(3002001);SRVR:BN1PR02MB072;BCL:0;PCL:0;RULEID:;SRVR:BN1PR02MB072; X-Forefront-PRVS: 059185FE08 X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 May 2015 18:20:37.0310 (UTC) X-MS-Exchange-CrossTenant-Id: fde4dada-be84-483f-92cc-e026cbee8e96 X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=fde4dada-be84-483f-92cc-e026cbee8e96;Ip=[165.204.84.222];Helo=[atltwp02.amd.com] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN1PR02MB072 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 5/29/2015 8:49 AM, Borislav Petkov wrote: > On Wed, May 27, 2015 at 02:03:35PM -0500, Aravind Gopalakrishnan wrote: >> >> +#define MAX_FLAG_OPT_SIZE 10 > Why 10? No specific reason. Just an arbitrary max value that we won't hit right now or in the future. > This should be 2 and increased when another, longer injection type > string gets introduced. Okay, I'll make it 2 here and will increase it to 3 in the next patch when I introduce injection types for the interrupts. > >> >> - *val = m->inject_flags; >> + for (i = 0; i <= N_INJ_TYPES; i++) { >> + inj_op = flags_options[i]; >> + if (inj_op && strncmp(inj_op, buf, cnt) == 0) >> + break; >> + } > What's wrong with: > > for (i = 0; i < N_INJ_TYPES; i++) { > if (!strncmp(flags_options[i], buf, strlen(flags_options[i]))) { > inj_type = i; > return 0; > } > } > > return -EINVAL; Hmm. That should work. Will simplify it in the next version. I think I had a NULL in flags_options[] to denote the invalid option. With your simplification, that can be removed. I'll make that change too. And the 'cnt' value that I used won't be necessary. So, will remove that from __set_inj() definition as well. >> +static ssize_t flags_write(struct file *filp, const char __user *ubuf, >> + size_t cnt, loff_t *ppos) >> +{ >> + char buf[MAX_FLAG_OPT_SIZE + 1]; >> + int err; >> + size_t ret; >> + >> + ret = cnt; > You're assigning cnt to ret here... > >> + >> + if (cnt > MAX_FLAG_OPT_SIZE) >> + cnt = MAX_FLAG_OPT_SIZE; > ... but correcting cnt afterwards. The assignment should be *after* that > correction. With the change to MAX_FLAG_OPT_SIZE to 2 (or 3 depending on the max size), this check is really an error check. Shall I make err = -EINVAL by default and return that here if the condition is not satisfied? Thanks, -Aravind.