Re: svn commit: r258142 - head/sys/net
On Thu, 14 Nov 2013, George Neville-Neil wrote: On Nov 14, 2013, at 16:03 , Sergey Kandaurov pluk...@freebsd.org wrote: On 15 November 2013 00:07, George V. Neville-Neil g...@freebsd.org wrote: Log: Shift our OUI correctly. Pointed out by: emaste Modified: head/sys/net/ieee_oui.h Modified: head/sys/net/ieee_oui.h == --- head/sys/net/ieee_oui.h Thu Nov 14 19:53:35 2013(r258141) +++ head/sys/net/ieee_oui.h Thu Nov 14 20:07:17 2013(r258142) @@ -62,5 +62,5 @@ */ /* Allocate 64K to bhyve */ -#define OUI_FREEBSD_BHYVE_LOW OUI_FREEBSD + 0x01 -#define OUI_FREEBSD_BHYVE_HIGH OUI_FREEBSD + 0x00 +#define OUI_FREEBSD_BHYVE_LOW ((OUI_FREEBSD 3) + 0x01) +#define OUI_FREEBSD_BHYVE_HIGH ((OUI_FREEBSD 3) + 0x00) This also fixes the syntax bugs (missing parentheses). Shouldn't it be rather shifted with 24 (3 x 8bits)? Correct, and it should really be an | not a +. I?ll commit a fix. Any chance of also fixing the style bugs? The most obvious ones are spaces instead of tabs after #define's. From the next commit: % Modified: head/sys/net/ieee_oui.h % == % --- head/sys/net/ieee_oui.h Thu Nov 14 21:31:58 2013(r258146) % +++ head/sys/net/ieee_oui.h Thu Nov 14 21:57:37 2013(r258147) % ... % @@ -62,5 +62,5 @@ % */ % % /* Allocate 64K to bhyve */ % -#define OUI_FREEBSD_BHYVE_LOW((OUI_FREEBSD 3) + 0x01) % -#define OUI_FREEBSD_BHYVE_HIGH ((OUI_FREEBSD 3) + 0x00) % +#define OUI_FREEBSD_BHYVE_LOW(((uint64_t)OUI_FREEBSD 24) | 0x01) % +#define OUI_FREEBSD_BHYVE_HIGH (((uint64_t)OUI_FREEBSD 24) | 0x00) % This introduces a new bug: uint64_t is used but isn't defined. To avoid all of the following design errors/style bugs: - requiring applications to include stdint.h to use the BHYVE part of this header - polluting this header for all applications by declaring uint64_t unconditionally in case the BHIVE parts are used - same as previous, but without (user-visible) pollution by using __uint64_t instead of uint64_t - breaking use of the definitions in cpp expressions by using any typedefed type in them - using the long long abomination instead of uint64_t in the cast to avoid the pollution and make the definition possibly useable in cpp expressions, though it may still have the wrong type - using the long long abomination as a type suffix for OUI_FREEBSD instead of casting OUI_FREEEBSD, I suggest #defining OUI_FREEBSD as some literal constant shifted right by 24. Its type is then determined by the C rules for types of literals combined with the rules for right shifts. Shifting OUI_FREEBSD back gives the same type as the original literal (this is not obvious) the same value as the original literal, and the same value as the left shift expressions above. (Except in cpp expressions types don't work normally.) Does the API require defining OUI_FREEBSD as a value that needs to be shifted left by 24 to use? It would be more useful to define it as the left-shifted value. Then it gives the base for the FreeBSD range. Values that aren't left shifted have the advantage of fitting in 32 bits, so that their natural type is always int the type needed to hold the left-shifted value can be implementation-defined (it should be int64_t on 32-bit systems and long on 64-bit systems). Bruce___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r258142 - head/sys/net
On Friday, November 15, 2013 7:23:23 am Bruce Evans wrote: On Thu, 14 Nov 2013, George Neville-Neil wrote: Does the API require defining OUI_FREEBSD as a value that needs to be shifted left by 24 to use? It would be more useful to define it as the left-shifted value. Then it gives the base for the FreeBSD range. Values that aren't left shifted have the advantage of fitting in 32 bits, so that their natural type is always int the type needed to hold the left-shifted value can be implementation-defined (it should be int64_t on 32-bit systems and long on 64-bit systems). The API is free to be whatever we want. I agree that it would be simpler to make the full value explicit. Perhaps something like this: #define OUI_FREEBSD_BASE 0x589cfc00 #define OUI_FREEBSD(nic) (OUI_FREEBSD_BASE | (nic)) And then you can have: #define OUI_FREEBSD_BHYVE_LOW OUI_FREEBSD(0x01) #define OUI_FREEBSD_BHYVE_HIGH OUI_FREEBSD(0x00) That avoids the needs for any casts, etc. -- John Baldwin ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
svn commit: r258142 - head/sys/net
Author: gnn Date: Thu Nov 14 20:07:17 2013 New Revision: 258142 URL: http://svnweb.freebsd.org/changeset/base/258142 Log: Shift our OUI correctly. Pointed out by: emaste Modified: head/sys/net/ieee_oui.h Modified: head/sys/net/ieee_oui.h == --- head/sys/net/ieee_oui.h Thu Nov 14 19:53:35 2013(r258141) +++ head/sys/net/ieee_oui.h Thu Nov 14 20:07:17 2013(r258142) @@ -62,5 +62,5 @@ */ /* Allocate 64K to bhyve */ -#define OUI_FREEBSD_BHYVE_LOW OUI_FREEBSD + 0x01 -#define OUI_FREEBSD_BHYVE_HIGH OUI_FREEBSD + 0x00 +#define OUI_FREEBSD_BHYVE_LOW ((OUI_FREEBSD 3) + 0x01) +#define OUI_FREEBSD_BHYVE_HIGH ((OUI_FREEBSD 3) + 0x00) ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r258142 - head/sys/net
On 15 November 2013 00:07, George V. Neville-Neil g...@freebsd.org wrote: Author: gnn Date: Thu Nov 14 20:07:17 2013 New Revision: 258142 URL: http://svnweb.freebsd.org/changeset/base/258142 Log: Shift our OUI correctly. Pointed out by: emaste Modified: head/sys/net/ieee_oui.h Modified: head/sys/net/ieee_oui.h == --- head/sys/net/ieee_oui.h Thu Nov 14 19:53:35 2013(r258141) +++ head/sys/net/ieee_oui.h Thu Nov 14 20:07:17 2013(r258142) @@ -62,5 +62,5 @@ */ /* Allocate 64K to bhyve */ -#define OUI_FREEBSD_BHYVE_LOW OUI_FREEBSD + 0x01 -#define OUI_FREEBSD_BHYVE_HIGH OUI_FREEBSD + 0x00 +#define OUI_FREEBSD_BHYVE_LOW ((OUI_FREEBSD 3) + 0x01) +#define OUI_FREEBSD_BHYVE_HIGH ((OUI_FREEBSD 3) + 0x00) Shouldn't it be rather shifted with 24 (3 x 8bits)? -- wbr, pluknet ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r258142 - head/sys/net
On Nov 14, 2013, at 16:03 , Sergey Kandaurov pluk...@freebsd.org wrote: On 15 November 2013 00:07, George V. Neville-Neil g...@freebsd.org wrote: Author: gnn Date: Thu Nov 14 20:07:17 2013 New Revision: 258142 URL: http://svnweb.freebsd.org/changeset/base/258142 Log: Shift our OUI correctly. Pointed out by: emaste Modified: head/sys/net/ieee_oui.h Modified: head/sys/net/ieee_oui.h == --- head/sys/net/ieee_oui.h Thu Nov 14 19:53:35 2013(r258141) +++ head/sys/net/ieee_oui.h Thu Nov 14 20:07:17 2013(r258142) @@ -62,5 +62,5 @@ */ /* Allocate 64K to bhyve */ -#define OUI_FREEBSD_BHYVE_LOW OUI_FREEBSD + 0x01 -#define OUI_FREEBSD_BHYVE_HIGH OUI_FREEBSD + 0x00 +#define OUI_FREEBSD_BHYVE_LOW ((OUI_FREEBSD 3) + 0x01) +#define OUI_FREEBSD_BHYVE_HIGH ((OUI_FREEBSD 3) + 0x00) Shouldn't it be rather shifted with 24 (3 x 8bits)? Correct, and it should really be an | not a +. I’ll commit a fix. Best, George signature.asc Description: Message signed with OpenPGP using GPGMail