GNOME Bugzilla – Bug 764663
Add abs() to double type
Last modified: 2016-04-14 07:05:12 UTC
Created attachment 325456 [details] [review] Patch Use the C functions fabs() to get the abs of a double in Vala.
Why the need for cheader_filename = "stdlib.h"? As far as I can tell fabs is part of math.h, which is already included in the VAPI. The C standard doesn't appear to be publically available, but Posix is: http://pubs.opengroup.org/onlinepubs/7908799/xsh/fabs.html
Created attachment 325640 [details] [review] Updated patch Updated patch based upon reviewer comments
Thanks for the update, looks good. For such a trivial patch I'm reluctant to point out it says [Patch 2/2]. If you have a moment and submit another then also in the GLib-2.0 bindings there appear to be groupings of bindings for each simple type, e.g. abs() for gshort is with the min(), max() and clamp() functions. Might be an idea to group abs for gdouble with min() and max() as well.
Created attachment 325666 [details] [review] Updated patch Updated based on reviewer comments
Review of attachment 325666 [details] [review]: Patch applies and works. Job done. Thanks for your perseverance and attention to detail.
You're welcome. Thank you for being patient and kind about it!
Created attachment 325800 [details] [review] glib-2.0: Add abs() to float and double
Review of attachment 325800 [details] [review]: Thanks for adding abs() for float as well, but both fabs() and fabsf() are defined in math.h. I know the C function abs() is defined in stdlib.h and maybe that is where the confusion comes from. From the GNU C library documentation, http://www.gnu.org/software/libc/manual/html_node/Absolute-Value.html , "Prototypes for abs, labs and llabs are in stdlib.h; imaxabs is declared in inttypes.h; fabs, fabsf and fabsl are declared in math.h" I marked the first accepted patch as commit_now so it could be picked up and committed, but as adding abs for float is a good idea maybe you could obsolete the first one to avoid commit_now patches accumulating.
Created attachment 325868 [details] [review] glib-2.0: Add abs() to float and double
Review of attachment 325666 [details] [review]: Replaced by adjusted patch
(In reply to Al Thomas from comment #8) > Review of attachment 325800 [details] [review] [review]: > > Thanks for adding abs() for float as well, but both fabs() and fabsf() are > defined in math.h. > I know the C function abs() is defined in stdlib.h and maybe that is where > the confusion comes > from. Yes, using math.h here is correct. Updated the patch
Review of attachment 325868 [details] [review]: Hmm, math.h is already included in the general headers for the binding of gfloat and gdouble. It's not necessary to have it again for the abs() bindings.
Attachment 325868 [details] pushed as 5c081e8 - glib-2.0: Add abs() to float and double