Skip to content

Commit

Permalink
Minor edits for C best practices (#66)
Browse files Browse the repository at this point in the history
* cleanup labels at end of compound statements

* "use" unused inputs

* Add compliance check for C99

* Define M_PI

* specify the c99 standard for cppcheck

* Let cppcheck see the headers as well

* Remove a file committed on accident

* Make the fact that `jd_tt` in refract.c is unused explicit

* Missing space

* -I is already included in CPPFLAGS

* Rephrase unused doc, style edits

* Doxygen comment on math.h

* Replace empty initializers with {0}

* Reword switch case fallthough comment to satisfy gcc

* More zero initializers

* Provide complete initialization for object structs

* Frame state should be unsigned to match the state constant default types

* fix the string fn name in `novas_make_frame` to match true name

* Fixed a bit of formatting
  • Loading branch information
kiranshila committed Sep 17, 2024
1 parent f39ccac commit dda3dcc
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 96 deletions.
2 changes: 1 addition & 1 deletion build.mk
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ distclean: distclean-local
.PHONY: check
check:
@echo " [check]"
@cppcheck $(CPPFLAGS) $(CHECKOPTS) src
@cppcheck $(CPPFLAGS) $(CHECKOPTS) $(SRC)

# Doxygen documentation (HTML and man pages) under apidocs/
.PHONY: dox
Expand Down
4 changes: 2 additions & 2 deletions config.mk
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ CC ?= gcc
CPPFLAGS += -I$(INC)

# Base compiler options (if not defined externally...)
CFLAGS ?= -Os -Wall
CFLAGS ?= -Os -Wall -std=c99

# Extra warnings (not supported on all compilers)
#CFLAGS += -Wextra
Expand Down Expand Up @@ -97,7 +97,7 @@ DEFAULT_SOLSYS ?= 3
# cppcheck options for 'check' target. You can add additional options by
# setting the CHECKEXTRA variable (e.g. in shell) prior to invoking 'make'.
CHECKOPTS ?= --enable=performance,warning,portability,style --language=c \
--error-exitcode=1 $(CHECKEXTRA)
--error-exitcode=1 --std=c99 $(CHECKEXTRA)

# Exhaustive checking for newer cppcheck...
#CHECKOPTS += --check-level=exhaustive
Expand Down
9 changes: 7 additions & 2 deletions include/novas.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,16 @@
#ifndef _NOVAS_
#define _NOVAS_

#include <math.h> // for M_PI
#include <math.h> // for sin, cos
#include <stdlib.h> // NULL
#include <stdint.h>
#include <time.h>

/// Definition of &pi; in case it's not defined in math.h
#ifndef M_PI
# define M_PI 3.14159265358979323846
#endif

// The upstream NOVAS library had a set of include statements that really were not necessary
// First, including standard libraries here meant that those libraries were included in the
// source code of any application that included 'novas.h', even if that source code did not
Expand Down Expand Up @@ -770,7 +775,7 @@ typedef struct {
* @see novas_change_observer()
*/
typedef struct {
int64_t state; ///< An internal state for checking validity.
uint64_t state; ///< An internal state for checking validity.
enum novas_accuracy accuracy; ///< NOVAS_FULL_ACCURACY or NOVAS_REDUCED_ACCURACY
novas_timespec time; ///< The instant of time for which this observing frame is valid
observer observer; ///< The observer location, or NULL for barycentric
Expand Down
4 changes: 2 additions & 2 deletions src/eph_manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,8 @@ short planet_ephemeris(const double tjd[2], enum de_planet target, enum de_plane
int do_earth = 0, do_moon = 0;

double jed[2];
double pos_moon[3] = { }, vel_moon[3] = { }, pos_earth[3] = { }, vel_earth[3] = { };
double target_pos[3] = { }, target_vel[3] = { }, center_pos[3] = { }, center_vel[3] = { };
double pos_moon[3] = {0}, vel_moon[3] = {0}, pos_earth[3] = {0}, vel_earth[3] = {0};
double target_pos[3] = {0}, target_vel[3] = {0}, center_pos[3] = {0}, center_vel[3] = {0};

if(!tjd || !position || !velocity)
return novas_error(-1, EINVAL, "planet_ephemeris", "NULL parameter: tjd=%p, position=%p, velocity=%p", tjd, position, velocity);
Expand Down
32 changes: 18 additions & 14 deletions src/frames.c
Original file line number Diff line number Diff line change
Expand Up @@ -306,9 +306,10 @@ static int is_frame_initialized(const novas_frame *frame) {
*/
int novas_make_frame(enum novas_accuracy accuracy, const observer *obs, const novas_timespec *time, double dx, double dy,
novas_frame *frame) {
static const char *fn = "novas_create_frame";
static const object earth = { NOVAS_PLANET, NOVAS_EARTH, "Earth" };
static const object sun = { NOVAS_PLANET, NOVAS_SUN, "Sun" };
static const char *fn = "novas_make_frame";
static const cat_entry zero_star = {0};
static const object earth = { NOVAS_PLANET, NOVAS_EARTH, "Earth", zero_star };
static const object sun = { NOVAS_PLANET, NOVAS_SUN, "Sun", zero_star };

double tdb2[2];
double mobl, tobl, ee, dpsi, deps;
Expand Down Expand Up @@ -769,18 +770,18 @@ int novas_app_to_hor(const novas_frame *frame, enum novas_reference_system sys,
switch(sys) {
case NOVAS_J2000:
matrix_transform(pos, &frame->precession, pos);
/* no break */
/* fallthrough */
case NOVAS_MOD:
matrix_transform(pos, &frame->nutation, pos);
/* no break */
/* fallthrough */
case NOVAS_TOD:
spin(15.0 * frame->gst, pos, pos);
break;

case NOVAS_ICRS:
case NOVAS_GCRS:
matrix_transform(pos, &frame->gcrs_to_cirs, pos);
/* no break */
/* fallthrough */
case NOVAS_CIRS:
spin(frame->era, pos, pos);
break;
Expand Down Expand Up @@ -941,15 +942,16 @@ int novas_app_to_geom(const novas_frame *frame, enum novas_reference_system sys,

case NOVAS_TOD:
matrix_inv_rotate(app_pos, &frame->nutation, app_pos);
/* no break */
/* fallthrough */
case NOVAS_MOD:
matrix_inv_rotate(app_pos, &frame->precession, app_pos);
/* no break */
/* fallthrough */
case NOVAS_J2000:
matrix_inv_rotate(app_pos, &frame->icrs_to_j2000, app_pos);
break;
default:
// nothing to do.
;
}

// Undo aberration correction
Expand Down Expand Up @@ -1044,26 +1046,27 @@ int novas_make_transform(const novas_frame *frame, enum novas_reference_system f
cat_transform(transform, &frame->gcrs_to_cirs, -1);
if(to_system == NOVAS_GCRS)
return 0;
/* no break */
/* fallthrough */

case NOVAS_GCRS:
cat_transform(transform, &frame->icrs_to_j2000, 1);
if(to_system == NOVAS_J2000)
return 0;
/* no break */
/* fallthrough */

case NOVAS_J2000:
cat_transform(transform, &frame->precession, 1);
if(to_system == NOVAS_MOD)
return 0;
/* no break */
/* fallthrough */

case NOVAS_MOD:
cat_transform(transform, &frame->nutation, 1);
return 0;

default:
// nothing to do...
;
}
}
else {
Expand All @@ -1072,26 +1075,27 @@ int novas_make_transform(const novas_frame *frame, enum novas_reference_system f
cat_transform(transform, &frame->nutation, -1);
if(to_system == NOVAS_MOD)
return 0;
/* no break */
/* fallthrough */

case NOVAS_MOD:
cat_transform(transform, &frame->precession, -1);
if(to_system == NOVAS_J2000)
return 0;
/* no break */
/* fallthrough */

case NOVAS_J2000:
cat_transform(transform, &frame->icrs_to_j2000, -1);
if(to_system == NOVAS_GCRS)
return 0;
/* no break */
/* fallthrough */

case NOVAS_GCRS:
cat_transform(transform, &frame->gcrs_to_cirs, 1);
return 0;

default:
// nothing to do...
;
}
}

Expand Down
45 changes: 25 additions & 20 deletions src/novas.c
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ int set_planet_provider_hp(novas_planet_provider_hp func) {
int place_star(double jd_tt, const cat_entry *star, const observer *obs, double ut1_to_tt, enum novas_reference_system system,
enum novas_accuracy accuracy, sky_pos *pos) {
static const char *fn = "place_star";
object source = { };
object source = {0};

if(!star || !pos)
return novas_error(-1, EINVAL, fn, "NULL input star=%p or output pos=%p pointer", star, pos);
Expand Down Expand Up @@ -741,7 +741,7 @@ int place_star(double jd_tt, const cat_entry *star, const observer *obs, double
*/
int radec_star(double jd_tt, const cat_entry *star, const observer *obs, double ut1_to_tt, enum novas_reference_system sys,
enum novas_accuracy accuracy, double *ra, double *dec, double *rv) {
sky_pos output = { };
sky_pos output = {0};

// Default return values in case of error.
if(ra)
Expand Down Expand Up @@ -804,7 +804,7 @@ int radec_star(double jd_tt, const cat_entry *star, const observer *obs, double
int radec_planet(double jd_tt, const object *ss_body, const observer *obs, double ut1_to_tt, enum novas_reference_system sys,
enum novas_accuracy accuracy, double *ra, double *dec, double *dis, double *rv) {
static const char *fn = "radec_planet";
sky_pos output = { };
sky_pos output = {0};

// Default return values in case of error.
if(ra)
Expand Down Expand Up @@ -1105,7 +1105,7 @@ short astro_planet(double jd_tt, const object *ss_body, enum novas_accuracy accu
short topo_star(double jd_tt, double ut1_to_tt, const cat_entry *star, const on_surface *position, enum novas_accuracy accuracy, double *ra,
double *dec) {
static const char *fn = "topo_star";
observer obs = { };
observer obs = {0};
prop_error(fn, make_observer(NOVAS_OBSERVER_ON_EARTH, position, NULL, &obs), 0);
prop_error(fn, radec_star(jd_tt, star, &obs, ut1_to_tt, NOVAS_TOD, accuracy, ra, dec, NULL), 0);
return 0;
Expand Down Expand Up @@ -1148,7 +1148,7 @@ short topo_star(double jd_tt, double ut1_to_tt, const cat_entry *star, const on_
short local_star(double jd_tt, double ut1_to_tt, const cat_entry *star, const on_surface *position, enum novas_accuracy accuracy,
double *ra, double *dec) {
static const char *fn = "local_star";
observer obs = { };
observer obs = {0};
prop_error(fn, make_observer(NOVAS_OBSERVER_ON_EARTH, position, NULL, &obs), 0);
prop_error(fn, radec_star(jd_tt, star, &obs, ut1_to_tt, NOVAS_GCRS, accuracy, ra, dec, NULL), 0);
return 0;
Expand Down Expand Up @@ -1196,7 +1196,7 @@ short local_star(double jd_tt, double ut1_to_tt, const cat_entry *star, const on
short topo_planet(double jd_tt, const object *ss_body, double ut1_to_tt, const on_surface *position, enum novas_accuracy accuracy,
double *ra, double *dec, double *dis) {
static const char *fn = "topo_planet";
observer obs = { };
observer obs = {0};
prop_error(fn, make_observer(NOVAS_OBSERVER_ON_EARTH, position, NULL, &obs), 0);
prop_error(fn, radec_planet(jd_tt, ss_body, &obs, ut1_to_tt, NOVAS_TOD, accuracy, ra, dec, dis, NULL), 0);
return 0;
Expand Down Expand Up @@ -1238,7 +1238,7 @@ short topo_planet(double jd_tt, const object *ss_body, double ut1_to_tt, const o
short local_planet(double jd_tt, const object *ss_body, double ut1_to_tt, const on_surface *position, enum novas_accuracy accuracy,
double *ra, double *dec, double *dis) {
static const char *fn = "local_planet";
observer obs = { };
observer obs = {0};
prop_error(fn, make_observer(NOVAS_OBSERVER_ON_EARTH, position, NULL, &obs), 0);
prop_error(fn, radec_planet(jd_tt, ss_body, &obs, ut1_to_tt, NOVAS_GCRS, accuracy, ra, dec, dis, NULL), 0);
return 0;
Expand Down Expand Up @@ -1272,7 +1272,7 @@ short local_planet(double jd_tt, const object *ss_body, double ut1_to_tt, const
*/
short mean_star(double jd_tt, double tra, double tdec, enum novas_accuracy accuracy, double *ira, double *idec) {
static const char *fn = "mean_star";
cat_entry star = { };
cat_entry star = {0};
double pos[3];
int iter;

Expand Down Expand Up @@ -1347,6 +1347,7 @@ short mean_star(double jd_tt, double tra, double tdec, enum novas_accuracy accur
int obs_posvel(double jd_tdb, double ut1_to_tt, enum novas_accuracy accuracy, const observer *obs, const double *geo_pos,
const double *geo_vel, double *pos, double *vel) {
static const char *fn = "get_obs_posvel";
static const cat_entry zero_star = {0};

if(!obs)
return novas_error(-1, EINVAL, fn, "NULL observer parameter");
Expand All @@ -1367,7 +1368,7 @@ int obs_posvel(double jd_tdb, double ut1_to_tt, enum novas_accuracy accuracy, co

if(!geo_pos || !geo_vel) {
const double tdb2[2] = { jd_tdb };
object earth = { NOVAS_PLANET, NOVAS_EARTH, "Earth" };
object earth = { NOVAS_PLANET, NOVAS_EARTH, "Earth", zero_star };
double gpos[3], gvel[3];
prop_error(fn, ephemeris(tdb2, &earth, NOVAS_BARYCENTER, accuracy, gpos, gvel), 0);
if(pos)
Expand All @@ -1389,7 +1390,7 @@ int obs_posvel(double jd_tdb, double ut1_to_tt, enum novas_accuracy accuracy, co
case NOVAS_OBSERVER_ON_EARTH:
case NOVAS_AIRBORNE_OBSERVER:
case NOVAS_OBSERVER_IN_EARTH_ORBIT: {
double pog[3] = { }, vog[3] = { };
double pog[3] = {0}, vog[3] = {0};
int i;

// For topocentric place, get geocentric position and velocity vectors
Expand All @@ -1407,6 +1408,7 @@ int obs_posvel(double jd_tdb, double ut1_to_tt, enum novas_accuracy accuracy, co

default:
// Nothing to do
;
}

return 0;
Expand Down Expand Up @@ -1492,7 +1494,7 @@ short place(double jd_tt, const object *source, const observer *location, double
static THREAD_LOCAL double peb[3], veb[3], psb[3];

observer obs;
novas_planet_bundle planets = {};
novas_planet_bundle planets = {0};
int pl_mask = (accuracy == NOVAS_FULL_ACCURACY) ? grav_bodies_full_accuracy : grav_bodies_reduced_accuracy;
double x, jd_tdb, pob[3], vob[3], pos[3] = { 0 }, vel[3], vpos[3], t_light, d_sb;
int i;
Expand Down Expand Up @@ -1659,6 +1661,7 @@ short place(double jd_tt, const object *source, const observer *location, double

default:
// Nothing else to do.
;
}

// ---------------------------------------------------------------------
Expand Down Expand Up @@ -3363,6 +3366,7 @@ short geo_posvel(double jd_tt, double ut1_to_tt, enum novas_accuracy accuracy, c
static THREAD_LOCAL double t_last = 0;
static THREAD_LOCAL enum novas_accuracy acc_last = -1;
static THREAD_LOCAL double gast;
static const cat_entry zero_star = {0};

double gmst, eqeq, pos1[3], vel1[3], jd_tdb, jd_ut1;

Expand Down Expand Up @@ -3437,7 +3441,7 @@ short geo_posvel(double jd_tt, double ut1_to_tt, enum novas_accuracy accuracy, c
}

case NOVAS_SOLAR_SYSTEM_OBSERVER: { // Observer in Solar orbit
const object earth = { NOVAS_PLANET, NOVAS_EARTH, "Earth" };
const object earth = { NOVAS_PLANET, NOVAS_EARTH, "Earth", zero_star };
const double tdb[2] = { jd_tdb, 0.0 };
int i;

Expand Down Expand Up @@ -3514,7 +3518,7 @@ int light_time2(double jd_tdb, const object *body, const double *pos_obs, double
static const char *fn = "light_time2";
int iter = 0;

double tol, jd[2] = { };
double tol, jd[2] = {0};

if(!tlight)
return novas_error(-1, EINVAL, fn, "NULL 'tlight' output pointer");
Expand Down Expand Up @@ -3893,9 +3897,10 @@ int obs_planets(double jd_tdb, enum novas_accuracy accuracy, const double *pos_o
*/
short grav_def(double jd_tdb, enum novas_observer_place unused, enum novas_accuracy accuracy, const double *pos_src, const double *pos_obs,
double *out) {
(void) unused;
static const char *fn = "grav_def";

novas_planet_bundle planets = {};
novas_planet_bundle planets = {0};
int pl_mask = (accuracy == NOVAS_FULL_ACCURACY) ? grav_bodies_full_accuracy : grav_bodies_reduced_accuracy;

if(!pos_src || !out)
Expand Down Expand Up @@ -3943,7 +3948,7 @@ short grav_def(double jd_tdb, enum novas_observer_place unused, enum novas_accur
*/
int grav_vec(const double *pos_src, const double *pos_obs, const double *pos_body, double rmass, double *out) {
static const char *fn = "grav_vec";
double pq[3], pe[3], pmag, emag, qmag, phat[3] = { }, ehat[3], qhat[3];
double pq[3], pe[3], pmag, emag, qmag, phat[3] = {0}, ehat[3], qhat[3];
int i;

if(!pos_src || !out)
Expand Down Expand Up @@ -4318,7 +4323,7 @@ double rad_vel2(const object *source, const double *pos_emit, const double *vel_
if(source->number > 0 && source->number < NOVAS_PLANETS)
rel *= (1.0 + zpl[source->number]);
}
/* no break */
/* fallthrough */

case NOVAS_EPHEM_OBJECT:
// Solar potential at source (bodies strictly outside the Sun's volume)
Expand Down Expand Up @@ -5412,7 +5417,7 @@ short cio_array(double jd_tdb, long n_pts, ra_of_cio *cio) {

// Check if it's a new file
if(last_file != cio_file) {
char line[80] = {};
char line[80] = {0};
int version, tokens;

last_file = NULL;
Expand Down Expand Up @@ -5627,7 +5632,7 @@ novas_ephem_provider get_ephem_provider() {
short ephemeris(const double *jd_tdb, const object *body, enum novas_origin origin, enum novas_accuracy accuracy, double *pos, double *vel) {
static const char *fn = "ephemeris";

double posvel[6] = { };
double posvel[6] = {0};
int error = 0;

if(!jd_tdb || !body)
Expand Down Expand Up @@ -5688,7 +5693,7 @@ short ephemeris(const double *jd_tdb, const object *body, enum novas_origin orig

// Check and adjust the origins as necessary.
if(origin != eph_origin) {
double pos0[3] = { }, vel0[3] = { };
double pos0[3] = {0}, vel0[3] = {0};
enum novas_planet refnum = (origin == NOVAS_BARYCENTER) ? NOVAS_SSB : NOVAS_SUN;
int i;

Expand Down Expand Up @@ -6475,7 +6480,7 @@ short make_observer(enum novas_observer_place where, const on_surface *loc_surfa
return novas_error(-1, EINVAL, fn, "NULL in space location (for velocity)");

memcpy(&obs->near_earth.sc_vel, loc_space->sc_vel, sizeof(loc_space->sc_vel));
/* no break */
/* fallthrough */

case NOVAS_OBSERVER_ON_EARTH:
if(!loc_surface)
Expand Down
Loading

0 comments on commit dda3dcc

Please sign in to comment.