See patch.
Uwe.
On Tue, Apr 08, 2008 at 02:41:23AM +0200, Uwe Hermann wrote:
+u8 bin2hex(u8 b) +{
- return "0123456789abcdef"[b & 15];
+}
Neat. :)
+u8 hex2bin(u8 h) +{
- return (('0' <= h && h <= '9') ? (h - '0') : \
('a' <= h && h <= 'f') ? (h - 'a' + 10) : 0);
+}
Might be worth mentioning in doxygen that the return value is 0 for invalid characters.
Also might be worth clarifying that these two only work on a nibble rather than a full byte.
//Peter
On 08/04/08 03:11 +0200, Peter Stuge wrote:
On Tue, Apr 08, 2008 at 02:41:23AM +0200, Uwe Hermann wrote:
+u8 bin2hex(u8 b) +{
- return "0123456789abcdef"[b & 15];
+}
Neat. :)
Hmm - now that you mention it - thats 16 bytes of const data. This might be more efficient (but not as cute):
return (b < 10) ? '0' + b : 'a' + (b - 10);
Jordan
On Mon, Apr 07, 2008 at 08:45:57PM -0600, Jordan Crouse wrote:
- return "0123456789abcdef"[b & 15];
Neat. :)
Hmm - now that you mention it - thats 16 bytes of const data. This might be more efficient (but not as cute):
return (b < 10) ? '0' + b : 'a' + (b - 10);
That probably comes close to 16 bytes of code though.
and 2 add 2 cmp 2 jle 2 add 2
10.. (IIRC the jump is just 2 byte when this short.)
Sure, the index and and make for 4 bytes too, but do the 10 bytes matter? :)
//Peter
On 08/04/08 02:41 +0200, Uwe Hermann wrote:
See patch.
Uwe.
http://www.hermann-uwe.de | http://www.holsham-traders.de http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
Convert BIN2HEX/HEX2BIN to functions and add the abs() family of functions while we're at it.
Signed-off-by: Uwe Hermann uwe@hermann-uwe.de
Index: include/libpayload.h
--- include/libpayload.h (Revision 3222) +++ include/libpayload.h (Arbeitskopie) @@ -41,10 +41,6 @@ #define MAX(a,b) ((a) > (b) ? (a) : (b)) #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
-#define BIN2HEX(b) ("0123456789abcdef"[b & 15]) -#define HEX2BIN(h) (('0' <= h && h <= '9') ? (h - '0') : \
('a' <= h && h <= 'f') ? (h - 'a' + 10) : 0)
#define LITTLE_ENDIAN 1234 #define BIG_ENDIAN 4321 #ifdef CONFIG_TARGET_I386 @@ -116,6 +112,11 @@ /* libc/lib.c */ int bcd2dec(int b); int dec2bcd(int d); +int abs(int j); +long int labs(long int j); +long long int llabs(long long int j); +u8 bin2hex(u8 b); +u8 hex2bin(u8 h);
/* libc/memory.c */ void *memset(void *s, int c, size_t n); Index: libc/lib.c =================================================================== --- libc/lib.c (Revision 3222) +++ libc/lib.c (Arbeitskopie) @@ -27,6 +27,8 @@
- SUCH DAMAGE.
*/
+#include <libpayload.h>
/*
- Convert a number in BCD format to decimal.
@@ -49,3 +51,34 @@ return ((d / 10) << 4) | (d % 10); }
+/**
- Return the absolute value of the specified integer.
- @param val The integer of which we want to know the absolute value.
- @return The absolute value of the specified integer.
- */
+int abs(int j) +{
- return (j >= 0 ? j : -j);
+}
+long int labs(long int j) +{
- return (j >= 0 ? j : -j);
+}
+long long int llabs(long long int j) +{
- return (j >= 0 ? j : -j);
+}
Do we really need all the types? I think that we can get by with just abs(), and probably even make it a #define which will eliminate the need for type checking anyway.
#define abs(_a) ((_a) >= 0 ? (_a) : -(_a))
Jordan
Do we really need all the types? I think that we can get by with just abs(), and probably even make it a #define which will eliminate the need for type checking anyway.
Only if you are really careful and make it a statement expression...
#define abs(_a) ((_a) >= 0 ? (_a) : -(_a))
...since this version ("the obvious version") suffers from double evaluation.
Segher
Updated patch with comments merged. Added doxygen comments, and hex2bin() now also supports upper-case characters.
On Tue, Apr 08, 2008 at 08:54:40AM +0200, Segher Boessenkool wrote:
Do we really need all the types? I think that we can get by
Dunno, I just needed an abs(), but when looking at the manpage I also saw that labs() and llabs() are part of a standard libc so I thought I might as well just add them too.
with just abs(), and probably even make it a #define which will eliminate the need for type checking anyway.
Maybe, but then we could also make the other mini-functions macros again, e.g. bin2hex() or most of libc/ctype.c or libc/string.c for example. I think the usual case should be to make small functions instead of macros.
Only if you are really careful and make it a statement expression...
#define abs(_a) ((_a) >= 0 ? (_a) : -(_a))
...since this version ("the obvious version") suffers from double evaluation.
What fix do you propose? Is there a way to improve the macro or do you suggest to leave the code as a function?
Uwe.
with just abs(), and probably even make it a #define which will eliminate the need for type checking anyway.
Maybe, but then we could also make the other mini-functions macros again, e.g. bin2hex() or most of libc/ctype.c or libc/string.c for example. I think the usual case should be to make small functions instead of macros.
Even if you want this inlined (and you don't), you can keep it a function -- make it "static inline" and put it in the header file. But you really don't want to, it will increase code size.
For the abs() stuff you might want it though, dunno. I doubt it would matter for speed reasons.
Only if you are really careful and make it a statement expression...
#define abs(_a) ((_a) >= 0 ? (_a) : -(_a))
...since this version ("the obvious version") suffers from double evaluation.
What fix do you propose? Is there a way to improve the macro or do you suggest to leave the code as a function?
Leave it as a function. If you _have_ to have it a macro, make it a statement expression (that's a GCC extension, read the Fine Manual -- "info gcc").
Segher
On 08/04/08 02:41 +0200, Uwe Hermann wrote:
See patch.
Uwe.
http://www.hermann-uwe.de | http://www.holsham-traders.de http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
Convert BIN2HEX/HEX2BIN to functions and add the abs() family of functions while we're at it.
Signed-off-by: Uwe Hermann uwe@hermann-uwe.de
Oops - I just acked the wrong patch. How embarassing. Really Acked-by: Jordan Crouse jordan.crouse@amd.com
Index: include/libpayload.h
--- include/libpayload.h (Revision 3222) +++ include/libpayload.h (Arbeitskopie) @@ -41,10 +41,6 @@ #define MAX(a,b) ((a) > (b) ? (a) : (b)) #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
-#define BIN2HEX(b) ("0123456789abcdef"[b & 15]) -#define HEX2BIN(h) (('0' <= h && h <= '9') ? (h - '0') : \
('a' <= h && h <= 'f') ? (h - 'a' + 10) : 0)
#define LITTLE_ENDIAN 1234 #define BIG_ENDIAN 4321 #ifdef CONFIG_TARGET_I386 @@ -116,6 +112,11 @@ /* libc/lib.c */ int bcd2dec(int b); int dec2bcd(int d); +int abs(int j); +long int labs(long int j); +long long int llabs(long long int j); +u8 bin2hex(u8 b); +u8 hex2bin(u8 h);
/* libc/memory.c */ void *memset(void *s, int c, size_t n); Index: libc/lib.c =================================================================== --- libc/lib.c (Revision 3222) +++ libc/lib.c (Arbeitskopie) @@ -27,6 +27,8 @@
- SUCH DAMAGE.
*/
+#include <libpayload.h>
/*
- Convert a number in BCD format to decimal.
@@ -49,3 +51,34 @@ return ((d / 10) << 4) | (d % 10); }
+/**
- Return the absolute value of the specified integer.
- @param val The integer of which we want to know the absolute value.
- @return The absolute value of the specified integer.
- */
+int abs(int j) +{
- return (j >= 0 ? j : -j);
+}
+long int labs(long int j) +{
- return (j >= 0 ? j : -j);
+}
+long long int llabs(long long int j) +{
- return (j >= 0 ? j : -j);
+}
+u8 bin2hex(u8 b) +{
- return "0123456789abcdef"[b & 15];
+}
+u8 hex2bin(u8 h) +{
- return (('0' <= h && h <= '9') ? (h - '0') : \
('a' <= h && h <= 'f') ? (h - 'a' + 10) : 0);
+}
-- coreboot mailing list coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
On Fri, Apr 11, 2008 at 11:30:52AM -0600, Jordan Crouse wrote:
Really Acked-by: Jordan Crouse jordan.crouse@amd.com
Thanks, r3235.
Uwe.