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 + 0x000001
-#define OUI_FREEBSD_BHYVE_HIGH OUI_FREEBSD + 0x00ffff
+#define OUI_FREEBSD_BHYVE_LOW ((OUI_FREEBSD << 3) + 0x000001)
+#define OUI_FREEBSD_BHYVE_HIGH ((OUI_FREEBSD << 3) + 0x00ffff)
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) + 0x000001)
% -#define OUI_FREEBSD_BHYVE_HIGH ((OUI_FREEBSD << 3) + 0x00ffff)
% +#define OUI_FREEBSD_BHYVE_LOW (((uint64_t)OUI_FREEBSD << 24) |
0x000001)
% +#define OUI_FREEBSD_BHYVE_HIGH (((uint64_t)OUI_FREEBSD << 24) |
0x00ffff)
%
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"
