From: Michal Nazarewicz <mina86@mina86.com>
To: Krzysztof Opasiak <k.opasiak@samsung.com>, "'Felipe Balbi'" <balbi@ti.com>,
        Krzysztof Opasiak <k.opasiak@samsung.com>
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCHv2 5/5] tools: ffs-test: add compatibility code for old kernels
In-Reply-To: <006601cf83bc$66197b30$324c7190$%opasiak@samsung.com>
Organization: http://mina86.com/
References: <1402045976-26580-1-git-send-email-mina86@mina86.com> <1402045976-26580-5-git-send-email-mina86@mina86.com> <006601cf83bc$66197b30$324c7190$%opasiak@samsung.com>
User-Agent: Notmuch/0.17+15~gb65ca8e (http://notmuchmail.org) Emacs/24.4.50.1 (x86_64-unknown-linux-gnu)
Face: iVBORw0KGgoAAAANSUhEUgAAADAAAAAwBAMAAAClLOS0AAAAJFBMVEWbfGlUPDDHgE57V0jUupKjgIObY0PLrom9mH4dFRK4gmjPs41MxjOgAAACQElEQVQ4jW3TMWvbQBQHcBk1xE6WyALX1069oZBMlq+ouUwpEQQ6uRjttkWP4CmBgGM0BQLBdPFZYPsyFUo6uEtKDQ7oy/U96XR2Ux8ehH/89Z6enqxBcS7Lg81jmSuujrfCZcLI/TYYvbGj+jbgFpHJ/bqQAUISj8iLyu4LuFHJTosxsucO4jSDNE0Hq3hwK/ceQ5sx97b8LcUDsILfk+ovHkOIsMbBfg43VuQ5Ln9YAGCkUdKJoXR9EclFBhixy3EGVz1K6eEkhxCAkeMMnqoAhAKwhoUJkDrCqvbecaYINlFKSRS1i12VKH1XpUd4qxL876EkMcDvHj3s5RBajHHMlA5iK32e0C7VgG0RlzFPvoYHZLRmAC0BmNcBruhkE0KsMsbEc62ZwUJDxWUdMsMhVqovoT96i/DnX/ASvz/6hbCabELLk/6FF/8PNpPCGqcZTGFcBhhAaZZDbQPaAB3+KrWWy2XgbYDNIinkdWAFcCpraDE/knwe5DBqGmgzESl1p2E4MWAz0VUPgYYzmfWb9yS4vCvgsxJriNTHoIBz5YteBvg+VGISQWUqhMiByPIPpygeDBE6elD973xWwKkEiHZAHKjhuPsFnBuArrzxtakRcISv+XMIPl4aGBUJm8Emk7qBYU8IlgNEIpiJhk/No24jHwkKTFHDWfPniR4iw5vJaw2nzSjfq2zffcE/GDjRC2dn0J0XwPAbDL84TvaFCJEU4Oml9pRyEUhR3Cl2t01AoEjRbs0sYugp14/4X5n4pU4EHHnMAAAAAElFTkSuQmCC
Date: Mon, 09 Jun 2014 15:27:58 +0200
Message-ID: <xa1tmwdmdxgh.fsf@mina86.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org

On Mon, Jun 09 2014, Krzysztof Opasiak wrote:
> As this is an example which will be copy-paste by many users maybe you
> should you struct usb_functionfs_descs_head and struct
> usb_functionfs_descs_head_v2 instead of direct operations using
> hard-coded offsets to make this function more readable?

v3 uses usb_functionfs_descs_head{_v2,} instead of hard-coded offsets.
I also started wondering if it would make sense to go one step further
and define a temporary structure holding the headers.  Something like:

----------- >8 ---------------------------------------------------------
	/* Read v2 header */
	{
		const struct {
			const struct usb_functionfs_descs_head_v2 header;
			const __le32 counts[];
		} __attribute__((packed)) *const in = descriptors;
		const __le32 *counts = in->counts;
		__u32 flags;

		if (le32_to_cpu(in->header.magic) !=
		    FUNCTIONFS_DESCRIPTORS_MAGIC_V2)
			return 0;
		length = le32_to_cpu(in->header.length);
		if (length <= sizeof in->header)
			return 0;
		length -= sizeof in->header;
		flags = le32_to_cpu(in->header.flags);
		if (flags & ~(FUNCTIONFS_HAS_FS_DESC | FUNCTIONFS_HAS_HS_DESC |
			      FUNCTIONFS_HAS_SS_DESC))
			return 0;

#define GET_NEXT_COUNT_IF_FLAG(ret, flg) do {		\
			if (!(flags & (flg)))		\
				break;			\
			if (length < 4)			\
				return 0;		\
			ret = le32_to_cpu(*counts);	\
			length -= 4;			\
			++counts;			\
		} while (0)

		GET_NEXT_COUNT_IF_FLAG(fs_count, FUNCTIONFS_HAS_FS_DESC);
		GET_NEXT_COUNT_IF_FLAG(hs_count, FUNCTIONFS_HAS_HS_DESC);
		GET_NEXT_COUNT_IF_FLAG(count, FUNCTIONFS_HAS_SS_DESC);

		count = fs_count + hs_count;
		if (!count)
			return 0;
		descs_start = descs_end = (const void *)counts;

#undef GET_NEXT_COUNT_IF_FLAG
	}

----------- >8 ---------------------------------------------------------

	/* Allocate legacy descriptors and copy the data. */
	{
		struct {
			struct usb_functionfs_descs_head header;
			__u8 descriptors[];
		} __attribute__((packed)) *out;

		length = sizeof out->header + (descs_end - descs_start);
		out = malloc(length);
		out->header.magic = cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC);
		out->header.length = cpu_to_le32(length);
		out->header.fs_count = cpu_to_le32(fs_count);
		out->header.hs_count = cpu_to_le32(hs_count);
		memcpy(out->descriptors, descs_start, descs_end - descs_start);
		*legacy = out;
	}
----------- >8 ---------------------------------------------------------

Thoughts?

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/