Change in ...coreboot[master]: include: Implement abs()

[Patrick Rudolph (Code Review)]

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32501

Change subject: include: Implement abs()
......................................................................

include: Implement abs()

There are a few external libraries that make use of this simple
function.

Change-Id: I0719de6558cf9224ecbbd46b6f8a8040e8225a79
Signed-off-by: Patrick Rudolph 
---
M src/include/stdlib.h
1 file changed, 5 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/32501/1

diff --git a/src/include/stdlib.h b/src/include/stdlib.h
index 34ef93a..a396a3b 100644
--- a/src/include/stdlib.h
+++ b/src/include/stdlib.h
@@ -11,4 +11,9 @@
 /* We never free memory */
 static inline void free(void *ptr) {}

+static inline int abs(int i)
+{
+	return i < 0 ? -i : i;
+}
+
 #endif /* STDLIB_H */

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32501 )

Change subject: include: Implement abs()
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/#/c/32501/3/src/include/stdlib.h 
File src/include/stdlib.h:

https://review.coreboot.org/#/c/32501/3/src/include/stdlib.h@14 
PS3, Line 14: static inline int abs(int i)
            : {
            : 	return i < 0 ? -i : i;
            : }
Would it not make more sense to implement it as a macro such that you can use different integer types?

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32501 )

Change subject: include: Implement abs()
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/#/c/32501/3/src/include/stdlib.h 
File src/include/stdlib.h:

https://review.coreboot.org/#/c/32501/3/src/include/stdlib.h@14 
PS3, Line 14: static inline int abs(int i)
            : {
            : 	return i < 0 ? -i : i;
            : }
> Would it not make more sense to implement it as a macro such that you can use different integer type […]

it's a function defined in C89. It might cause issues if it does not return an int.

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32501 )

Change subject: include: Implement abs()
......................................................................

Patch Set 3:

There is an abs() in commonlib/helpers.h, please base this off that.

Change in coreboot[master]: include: Implement abs()

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32501 )

Change subject: include: Implement abs()
......................................................................

Patch Set 3:
> Patch Set 3:
>
> There is an abs() in commonlib/helpers.h, please base this off that.

I tried to use ABS instead of abs(), but it doesn't work as the 3rdparty library depends on the C89 function signature and the casts implicitly done usingthis function.

Change in coreboot[master]: include: Implement abs()

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32501 )

Change subject: include: Implement abs()
......................................................................

Patch Set 3:
> I tried to use ABS instead of abs(), but it doesn't work as the 3rdparty library depends on the C89 function signature and the casts implicitly done usingthis function.

If it needs to be quirky to support specific vendorcode, can you define it in a special wrapper header for that vendorcode rather than in generic coreboot headers? If we have two different abs() implementations people are bound to start using the wrong one eventually, and I'd expect that in most cases the fact that it truncates to int is more likely to cause than to prevent a problem.

Change in coreboot[master]: include: Implement abs()

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32501 )

Change subject: include: Implement abs()
......................................................................

Patch Set 3:
> Patch Set 3:
> > I tried to use ABS instead of abs(), but it doesn't work as the 3rdparty library depends on the C89 function signature and the casts implicitly done usingthis function.
>
> If it needs to be quirky to support specific vendorcode, can you define it in a special wrapper header for that vendorcode rather than in generic coreboot headers? If we have two different abs() implementations people are bound to start using the wrong one eventually, and I'd expect that in most cases the fact that it truncates to int is more likely to cause than to prevent a problem.

will do.

Change in coreboot[master]: include: Implement abs()

[Patrick Rudolph (Code Review)]

Patrick Rudolph has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/32501 )

Change subject: include: Implement abs()
......................................................................

Abandoned

will be implemented in 3rdparty headers