linux-kernel - Re: [PATCH] mtd: ubi: Extend UBI layer debug/messaging capabilities

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 30 Sep 2014 11:02:23 +0300
From:	Tanya Brokhman <tlinder@...eaurora.org>
To:	dedekind1@...il.com
CC:	dedeking1@...il.com, linux-mtd@...ts.infradead.org,
	linux-arm-msm@...r.kernel.org,
	David Woodhouse <dwmw2@...radead.org>,
	Brian Norris <computersforpeace@...il.com>,
	Richard Weinberger <richard@....at>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mtd: ubi: Extend UBI layer debug/messaging capabilities

On 9/28/2014 3:01 PM, Artem Bityutskiy wrote:
> On Sun, 2014-09-28 at 09:36 +0300, Tanya Brokhman wrote:
>> If there is more then one UBI device mounted, there is no way to
>> distinguish between messages from different UBI devices.
>> Add device number to all ubi layer message types.
>
> Hi, the goal looks legit to me, but the patch is so large that I do not
> think that I can really review it in this form.
>
> a) A patch which changes the macros (ubi_err(), etc)

If I divide the patches like this, patch (a) wont compile

> b) A set of patches which do not change messages at all, but add the
> 'ubi' parameter to the places where it is missing.
> c) A patch which changes the messages.

I think patches (b)+(C) can be combined into one patch. Don't you agree? 
I changed ~2 or 3 messages that were printing ubi number by themselves. 
No need for a separate patch for this. Don't you agree?

>
> So a) will be the most important patch for the reviewer. b) - more or
> less mechanical changes of a similar kind. c) - the same.
>
> Also, if you add a parameter to 'ubi_err()' and the other printing
> wrappers, add 'ubi' there, not 'ubi_num'. This will allow to prefix
> messages with vary different things, not just the device number in the
> future. So the calls would look like
>
> ubi_err(ubi, "inconsistent used_ebs");
>
> Once this is done, the series should be more reviewable. The next thing
> I'd check is whether we really need to change all the messages, or most
> of them, or we actually need to change only a small part of them. In the
> former case, it is OK to do what you do, I guess. In the latter case we
> probably better off with introducing a separate set of printing macros
> and leave the existing ones as they are.

Large portion of the messages needs updating. I think perhaps I'll 
overcome the messages during init (when I don't' have the ubi yet) in a 
different manner and add "ubi" not "ubi_num" parameter to the macros, as 
you suggested

>
> Thanks!
>


-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/