Re: [PATCH] hcidump: add support for time64 based libc

[Guy Harris]

On Jan 10, 2020, at 12:49 PM, Arnd Bergmann <arnd@xxxxxxxx> 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.