This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
Craig, Thanks for the review! On 9/23/14 7:17 PM, Craig Howland wrote:
Oops. I'm not very familiar with Newlib processes, so please let me know if I'm missing other things that you'd normally expect from such an upstream submission.... For example, some of my co-workers reminded me to also update COPYING.NEWLIB, and add copyright headers to the new files.On 09/23/2014 04:33 PM, Jonathan Roelofs wrote:Please see the attached patch.Needs a ChangeLog entry. Missing the patch for libm/common/Makefile.am.
Operationally, all looks OK, but aesthetically, the use of casting is inconsistent. For example: +long double +nexttowardl (long double x, long double y) +{ + return nextafter((double)x, (double)y); +} The x and y arguments are cast to double to pass to nextafter(), but the return value is not cast to long double. It works OK, but could be confusing. Are the casts intended to highlight the spots where the double/long double equality is required?
Yeah, that was my intent.
If so, it would probably be better to either cast in all places where it is theoretically needed, or to just leave them out. The other existing functions of this style left all of them out (i.e. no casts from ld to d or d to ld)--see, for example, libm/common/nextafterl.c. I suggest that the precedent is the better way.
Agreed, consistency is probably better.Attached is a new version of this patch with these things addressed, and I've added another similar shim for log2l.
Cheers, Jon
Craig
-- Jon Roelofs jonathan@codesourcery.com CodeSourcery / Mentor Embedded
Attachment:
newlib_nexttoward2.diff
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |