linux-kernel - Re: [PATCH v1] firmware loader: introduce module parameter to customize fw search path

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+55aFwaLnyYFaydxw1ALmp_EP217DmgY=ihNy3z1ZqF-pqkSA@mail.gmail.com>
Date:	Thu, 25 Oct 2012 19:32:01 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Ming Lei <ming.lei@...onical.com>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] firmware loader: introduce module parameter to
 customize fw search path

On Thu, Oct 25, 2012 at 5:46 PM, Ming Lei <ming.lei@...onical.com> wrote:
>                 struct file *file;
> -               snprintf(path, PATH_MAX, "%s/%s", fw_path[i], buf->fw_id);
> +
> +               if (i < 0) {
> +                       if (!fw_path_para[0])   /* No customized path */
> +                               continue;
> +                       snprintf(path, PATH_MAX, "%s/%s", fw_path_para,
> +                                buf->fw_id);
> +               } else {
> +                       snprintf(path, PATH_MAX, "%s/%s", fw_path[i],
> +                                buf->fw_id);
> +               }

Ugh. This is just disgusting.

Please just make "fw_path[0]" just be the pointer to fw_path_para[]
(which sounds like the cleanest fix) and get rid of the negative 'i'
and conditional entirely.

Or if there is some odd reason you don't want to do that, at least
make the conditional much smaller, without the snprintf() in both arms
(ie make the if-statement just set a "const char *dir" variable to
either fw_path[i] or fw_path_para or whatever).

Sure, the compiler *may* merge them (gcc does, but I've seen it miss
them too), but even if the compiler might fix up ugly code, that's not
a reason for it to be ugly in the source code anyway.

          Linus
--
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/