Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3608.40.2.2.4\))
Subject: Re: [PATCH] hcidump: add support for time64 based libc
From:   Guy Harris <guy@alum.mit.edu>
In-Reply-To: <20200110204903.3495832-1-arnd@arndb.de>
Date:   Fri, 10 Jan 2020 15:30:16 -0800
Cc:     Bluez mailing list <linux-bluetooth@vger.kernel.org>,
        y2038 Mailman List <y2038@lists.linaro.org>,
        Johan Hedberg <johan.hedberg@gmail.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Deepa Dinamani <deepa.kernel@gmail.com>,
        Rich Felker <dalias@libc.org>
Content-Transfer-Encoding: 8BIT
Message-Id: <25105A85-A3EE-4E6A-AAF5-D54FCF9E6ACB@alum.mit.edu>
References: <20200110204903.3495832-1-arnd@arndb.de>
To:     Arnd Bergmann <arnd@arndb.de>
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

On Jan 10, 2020, at 12:49 PM, Arnd Bergmann <arnd@arndb.de> wrote:

> diff --git a/monitor/hcidump.c b/monitor/hcidump.c
> index 8b6f846d3..6d2330287 100644
> --- a/monitor/hcidump.c
> +++ b/monitor/hcidump.c
> @@ -107,6 +107,36 @@ static int open_hci_dev(uint16_t index)
> 	return fd;
> }
> 
> +static struct timeval hci_tstamp_read(void *data)
> +{
> +	struct timeval tv;
> +
> +	/*
> +	 * On 64-bit architectures, the data matches the timeval
> +	 * format. Note that on sparc64 this is different from
> +	 * all others.
> +	 */
> +	if (sizeof(long) == 8) {
> +		memcpy(&tv, data, sizeof(tv));
> +	}
> +
> +	/*
> +	 * On 32-bit architectures, the timeval definition may
> +	 * use 32-bit or 64-bit members depending on the C
> +	 * library and architecture.
> +	 * The cmsg data however always contains a pair of
> +	 * 32-bit values. Interpret as unsigned to make it work
> +	 * past y2038.
> +	 */
> +	if (sizeof(long) == 4) {
> +		unsigned int *stamp = data;
> +		tv.tv_sec = stamp[0];
> +		tv.tv_usec = stamp[1];
> +	}
> +
> +	return tv;
> +}

Should it be something more like

	if (sizeof(long) == 8) {
		/*
		 * On 64-bit architectures, the data matches the timeval
		 * format. Note that on sparc64 this is different from
		 * all others.
		 */
		memcpy(&tv, data, sizeof(tv));
	} else if (sizeof(long) == 4) {
		/*
		 * On 32-bit architectures, the timeval definition may
		 * use 32-bit or 64-bit members depending on the C
		 * library and architecture.
		 * The cmsg data however always contains a pair of
		 * 32-bit values. Interpret as unsigned to make it work
		 * past y2038.
		 */
		unsigned int *stamp = data;
		tv.tv_sec = stamp[0];
		tv.tv_usec = stamp[1];
	} else {
		abort();	/* or some other "sorry, we're not ready for 128-bit or weird architectures yet" failure */
	}

	return tv;

> static void device_callback(int fd, uint32_t events, void *user_data)
> {
> 	struct hcidump_data *data = user_data;
> @@ -150,7 +180,7 @@ static void device_callback(int fd, uint32_t events, void *user_data)
> 				memcpy(&dir, CMSG_DATA(cmsg), sizeof(dir));
> 				break;
> 			case HCI_CMSG_TSTAMP:
> -				memcpy(&ctv, CMSG_DATA(cmsg), sizeof(ctv));
> +				ctv = hci_tstamp_read(CMSG_DATA(cmsg));
> 				tv = &ctv;
> 				break;
> 			}

And libpcap's Linux BT code should do the same thing, changing its memcpy() call?

If you want, you can submit a pull request, or I can make the change.