linux-kernel - Re: [PATCH v2 1/7] lib: string: add functions to case-convert strings

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: <87oa692d8e.fsf@rasmusvillemoes.dk>
Date:	Fri, 08 Jul 2016 02:19:13 +0200
From:	Rasmus Villemoes <linux@...musvillemoes.dk>
To:	Markus Mayer <mmayer@...adcom.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Al Viro <viro@...iv.linux.org.uk>,
	Chris Metcalf <cmetcalf@...hip.com>,
	Kees Cook <keescook@...omium.org>,
	dri-devel@...ts.freedesktop.org, nouveau@...ts.freedesktop.org,
	linux-acpi@...r.kernel.org, speakup@...ux-speakup.org,
	devel@...verdev.osuosl.org, linux-scsi@...r.kernel.org,
	target-devel@...r.kernel.org, linux-pm@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/7] lib: string: add functions to case-convert strings

On Tue, Jul 05 2016, Markus Mayer <mmayer@...adcom.com> wrote:

> Add a collection of generic functions to convert strings to lowercase
> or uppercase.
>
> Changing the case of a string (with or without copying it first) seems
> to be a recurring requirement in the kernel that is currently being
> solved by several duplicated implementations doing the same thing. This
> change aims at reducing this code duplication.
>
> +/**
> + * strncpytoupper - Copy a length-limited string and convert to uppercase.
> + * @dst: The buffer to store the result.
> + * @src: The string to convert to uppercase.
> + * @len: Maximum string length. May be 0 to set no limit.
> + *
> + * Returns pointer to terminating '\0' in @dst.
> + */
> +char *strncpytoupper(char *dst, const char *src, size_t len)
> +{
> +	size_t i;
> +
> +	for (i = 0; src[i] != '\0' && (i < len || !len); i++)
> +		dst[i] = toupper(src[i]);
> +	if (i < len || !len)
> +		dst[i] = '\0';
> +
> +	return dst + i;
> +}

Hm, this seems to copy the insane semantics from strncpy of not
guaranteeing '\0'-termination.

Why use 0 as a sentinel, when (size_t)-1 == SIZE_MAX would work just as
well and require a little less code (no || !len)?

I regret suggesting this return semantics and now agree that void would
be better, especially since there doesn't seem to be anyone who can
use this (or any other) return value. How about

if (!len)
   return;

for (i = 0; i < len && src[i]; ++i)
  dst[i] = toupper(src[i]);
dst[i < len ? i : i-1] = '\0';

(I think you must do i < len before testing src[i], since the len
parameter should be an upper bound on the number of bytes to access in
both src and dst).

Rasmus