Bug #1028

HAVE_BUGGY_SINCOS test is wrong

Added by Remko 10 months ago. Updated 6 months ago.

Status:ClosedStart date:2017-01-15
Priority:NormalDue date:
Assignee:Remko% Done:

100%

Category:-
Target version:Candidate for next bugfix release
Affected version:5.x-svn Platform:

Description

See HAVE_BUGGY_SINCOS from Peter
Regarding cmake/modules/ConfigureChecks.cmake

First the "#" should clearly be in front of the line

include <math.h>"

Second, the result is "1" when sincos works (see the comment "/* return TRUE if sincos works ok */"

The check clearly is meant to detect a sincos function that doesn't change c or s ... this may have been the case for some implementation on sincos on some platform.
Normally, it should return "1" on FAILURE, so the we need this line instead

return (s == s1 || c == c1);} /* return TRUE if sincos fails */

Associated revisions

Revision 17447
Added by Remko 10 months ago

This fixes issue #1028.

Revision 17448
Added by Remko 10 months ago

Merge r17447 from ^/trunk, which fixes issue #1028.

History

#1 Updated by Remko 10 months ago

  • Status changed from New to Resolved
  • Assignee set to Remko
  • % Done changed from 0 to 100

#2 Updated by Florian 10 months ago

Should we just remove this check altogether? This is an ancient leftover from the autotools configure checks. Are you aware of any faulty implementations of sincos?

#3 Updated by Paul 10 months ago

This dates way back to when some alpha chip(?) systems had a buggy sincos. It is at least 10 years and was added by Lloyd Parkes in New Zealand. I am sure it could be removed. Lloyd has not been active with GMT for probably 5 years and has not registered with the wiki.

#4 Updated by Paul 10 months ago

  • Status changed from Resolved to In Progress

See issue # 1033. Now we get SEGV under WIndows 10 Ubuntu as well. Something was broken in this fix.

#5 Updated by Paul 10 months ago

Under Ubuntu on Windows 10 the revised test says SINCOS is buggy and then our sincos replacement code is compiled. Yet, sincos exists and for unclear reasons when sincos is called we do enter into the code in gmt_notposix.c and it crashes. Not clear in ddd why it crashes. I think the fix needs to be revised. I agree with the missing hashtag in front of the include but not with the reversal of the test. I have put back the test the way it was. FYI, when I try that dummy program with sincos under WIn10 Ubuntu the results are correct. Yet, after the fix cmake says sincos is buggy.... Update in r17487.

#6 Updated by Paul 6 months ago

  • Status changed from In Progress to Resolved

I have made the decision to yank the HAVE_BUGGY_SINCOS entirely. If anyone is still using their 1999 Alpha-chip DEC station then they can send me an angry fax. In r18345.

#7 Updated by Paul 6 months ago

  • Status changed from Resolved to Closed

And shutting the door as well.

Also available in: Atom PDF