Skip to content

Commit 1e15ae7

Browse files
committed
alignment/SPK: address code review feedback
Fix undefined behavior in TransformTelescopeToCelestial: spkVtel reads tar->a and tar->b unconditionally before its internal mode switch, so leaving them uninitialized in TARG mode was UB. Initialize both to 0.0. Check the spkAstr return code in UpdateAstrometry. Zero m_Ast before the call so a failure cannot leave stale data behind; on error, seed eral from the libnova LST so subsequent spkVtel calls see a coherent ERA rather than garbage, then return early. Restore pmfit.c closer to the original vendored source.
1 parent ea5bb05 commit 1e15ae7

3 files changed

Lines changed: 45 additions & 28 deletions

File tree

libs/alignment/SPKMathPlugin.cpp

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,8 @@ bool SPKMathPlugin::TransformTelescopeToCelestial(const TelescopeDirectionVector
220220

221221
spkTAR tar;
222222
tar.sys = APPT;
223+
tar.a = 0.0; // not used by spkVtel in TARG mode, but read unconditionally before the mode switch
224+
tar.b = 0.0;
223225

224226
double tara, tare, tarr, tarp, soln[5];
225227
int status = spkVtel(TARG, &m_Obs, &m_Opt, &m_PM, &m_Ast, &ax3, &tar, &m_PO,
@@ -267,20 +269,32 @@ void SPKMathPlugin::UpdateAstrometry(double JD)
267269
utc.mi = date.minutes;
268270
utc.sec = date.seconds;
269271

272+
// Compute LST first — needed both for the ERA synchronization below and
273+
// as a safe fallback if spkAstr fails.
274+
double gmst_hrs = ln_get_apparent_sidereal_time(JD);
275+
double lst_rad = HOURS_TO_RAD(range24(gmst_hrs + RAD_TO_HOURS(m_Obs.slon)));
276+
m_LST_rad = lst_rad;
277+
270278
spkEOP eop = {0, 0, 0};
271279
// pressure=0 disables spkVtel's internal refraction model; temperature and humidity
272280
// are nominal placeholders. Atmospheric refraction is not modeled in this pipeline
273281
// (INDI's coordinate flow stops at "apparent" / JNow; see tar.sys = APPT above).
274282
spkAIR air = {0.0, 10.0, 0.5};
275-
276-
// Use official spkAstr
277-
spkAstr(1, utc, 0.0, &eop, &m_Obs, &air, &m_Opt, &m_Ast);
278283

279-
// Synchronize SOFA internal "clock" (ERA) with libnova LST
284+
// Zero m_Ast before calling spkAstr so a failure cannot leave stale data behind.
285+
memset(&m_Ast, 0, sizeof(m_Ast));
286+
int astrStatus = spkAstr(1, utc, 0.0, &eop, &m_Obs, &air, &m_Opt, &m_Ast);
287+
if (astrStatus < 0)
288+
{
289+
ASSDEBUGF("SPK UpdateAstrometry: spkAstr failed (status=%d), transforms will be inaccurate", astrStatus);
290+
// Leave m_Ast zeroed; set eral from libnova LST so subsequent spkVtel
291+
// calls see a minimally coherent ERA rather than garbage.
292+
m_Ast.astrom.eral = iauAnp(lst_rad);
293+
return;
294+
}
295+
296+
// Synchronize SOFA internal "clock" (ERA) with libnova LST.
280297
// This ensures consistency with the simulator's view of HA/Az.
281-
double gmst_hrs = ln_get_apparent_sidereal_time(JD);
282-
double lst_rad = HOURS_TO_RAD(range24(gmst_hrs + RAD_TO_HOURS(m_Obs.slon)));
283-
m_LST_rad = lst_rad;
284298
// eral = ERA + Longitude = LST + EO
285299
m_Ast.astrom.eral = iauAnp(lst_rad + m_Ast.eo);
286300
}

libs/alignment/spk/pmfit.c

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,7 @@
11
#include <stdio.h>
2-
#include <math.h>
32
#include <stdlib.h>
3+
#include <math.h>
44
#include <string.h>
5-
#include <stddef.h>
6-
#include "sofa.h"
7-
#include "spk.h"
8-
9-
int Bfun ( int, double, char, double, double, double* );
10-
int Simeqn (int, double*, double* );
11-
12-
int Pmfit ( double, char, int, double*, int,
13-
double*, double*, double* );
14-
15-
/* Internal helper Bfun prototype */
16-
int Bfun ( int, double, char, double, double, double* );
17-
int Simeqn (int, double*, double* );
18-
19-
/*--------------------------------------------------------------------*/
20-
215
#include <sofa.h>
226

237
int Bfun ( int, double, char, double, double, double* );
@@ -253,7 +237,7 @@ int Pmfit ( double phi, char mount, int n, double* obs, int nt,
253237
}
254238

255239
/* On-sky variance and RMS. */
256-
skyvar = ( 2*n > nt ) ? sumr2 / (double) ( 2*n - nt ) : 0.0;
240+
skyvar = sumr2 / (double) (n-nt);
257241

258242
/* Standard deviations of the model coefficients. */
259243
for ( i = 0; i < nt; i++ ) sigmas[i] = sqrt ( skyvar * a[i*(nt+1)] );
@@ -410,6 +394,27 @@ int Bfun ( int nt, double phi, char mount, double rdem, double pdem,
410394
** ---------------------------------------------------
411395
** The basis functions at the current point in the sky
412396
** ---------------------------------------------------
397+
**
398+
** Each term is an [x,y] correction vector at the given point in the
399+
** sky, where the x-axis is east-west for equatorials or left-right
400+
** for altazimuths and the y-axis north-south or up-down respectively.
401+
**
402+
** The equatorial terms are from Appendix 1 of Wallace, P.T., 1975,
403+
** Programming the Control Computer of the Anglo-Australian 3.9 Metre
404+
** Telescope. In Telescope Automation: proceedings of an International
405+
** Conference held at MIT, Cambridge, MA USA, April 29 - May 1, 1975,
406+
** Eds. Maureen K. Huguenin and Thomas B. McCord. Published by MIT,
407+
** 1975, p.281.
408+
**
409+
** The altazimuth terms are from Table 3 in Jeffrey G. Mangum,
410+
** Jacob W. M. Baars, Albert Greve, Robert Lucas, Ralph C. Snel,
411+
** Patrick Wallace, Mark Holdaway, 2006, Evaluation of the ALMA
412+
** Prototype Antennas, Publications of the Astronomical Society of the
413+
** Pacific, 118, 847, 1260.
414+
**
415+
** n.b. The formulas are first-order approximations, not rigorous frame
416+
** rotations, valid for relatively small values of the pointing-model
417+
** coefficients.
413418
*/
414419

415420
switch ( (int) mount ) {
@@ -608,4 +613,4 @@ int Simeqn (int n, double *a, double *y )
608613
** NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
609614
** CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
610615
**
611-
**--------------------------------------------------------------------*/
616+
**--------------------------------------------------------------------*/

libs/alignment/spk/vtel.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
#include <math.h>
2-
#include <string.h>
31
#include "sofa.h"
42
#include "spk.h"
53

0 commit comments

Comments
 (0)