Member Avatar for Nholdamek

I've been building this program to interface with the Swis ephemeris library.
It's working beautifully, except for one thing.
OK, I need to be able to specify it to retrieve information about multiple planets, such as longitude, speed, etc. So I have a function that parses the command line parameter for --planets and the like, and puts it into an array. The array is resized 5 elements at a time.
That all seems to be working fine, except that when I put it over 25 planets (i has stars and asteroids too, included), it starts having memory access errors. It's within the library, but I don't think it is the fault of the library since it is only when the program tries to retrieve information about more than 25 planets at once.
I'm going to attach the code. I really don't know which part of the code to post, but functions of interest include parsePlanets and printPlanets.
Thanks for any help.

/**
 * \file astrology.c
 * \brief Retrieves astrological data from the Swiss Ephemeris.
 *
 * \author	Brandon Olivares <nholdamek@spiritmagick.com>
 * \version:	1.0.0
 */

/*
 * Header files
 */

#include <stdio.h>		/** Standard input and output */

#include <stdlib.h>		/** Standard library */

#include <time.h>		/** Time functions */

#include <getopt.h>		/** Parsing command line arguments */

#include "swephexp.h"		/** Swiss ephemeris library */

/*
 * Constants
 */

/**
 * \brief Program version
 */
#define VERSION             "1.0.0"

/**
 * \brief Constant used to mark the end of a planets array.
 *
 * This is used so the program knows when it has looped through all of the
 * planets.
 *
 * The array for each type of planet, including major planets, fictitious
 * planets, stars, and asteroids, is originally allocated to 20 bytes, or 5
 * integers (4 bytes per integer).
 *
 * Generally, unless the number of planets is a multiple of five, there are
 * some slots left over. These slots are set to END_OF_PLANETS, which is
 * hexadecimal 7F7F7F7F.
 */
#define END_OF_PLANETS      2139062143

/*
 * Bitflags
 */

/**
 * \brief Flag used when the user wants to retrieve the longitude
 */
#define LONGITUDE 1

/**
 * \brief Flag used when the user wants to retrieve the latitude of planets
 */
#define LATITUDE  2

/**
 * \brief Flag used when the user wants to retrieve the speed of planets
 */
#define SPEED     4

/**
 * \brief Seconds in a week
 *
 * \sa findDegree()
 */
#define SECONDS_IN_WEEK 604800

/**
 * \brief Seconds in a day
 *
 * \sa findDegree()
 */
#define SECONDS_IN_DAY 86400

/**
 * \brief Seconds in an hour
 *
 * \sa findDegree()
 */
#define SECONDS_IN_HOUR 3600

/**
 * \brief Seconds in a minute
 *
 * \sa findDegree()
 */
#define SECONDS_IN_MINUTE 60

/*
 * global variables
 */

/**
 * \brief Array of asteroids
 */
int *asteroids = NULL;

/**
 * \brief Array of fictitious planets
 */
int *fictitious_planets = NULL;

/**
 * \brief Array of major planets
 */
int *planets = NULL;

/**
 * \brief Array of stars
 */
int *stars = NULL;

/**
 * \brief Pointer to instance of tm structure for date/time data
 */
struct tm *tm = NULL;

/**
 * \brief Bitflags to specify information to retrieve
 */
int bitflags = 0;

/**
 * \brief Is the date set from the command line?
 */
int date_set = 0;

/**
 * \brief Day part of date
 */
int day = 0;

/**
 * \brief Whether to find when planet passes through
 * degree; -1 = no, any other number = degree to
 * find
 */
double find_degree = -1;

/**
 * \briefFlags for swe_calc_ut() and the like
 */
int flags = 0;

/**
 * \brief Timestamp representing date/time
 */
time_t gmttime = 0;

/**
 * \brief Hour part of time
 */
int hour = 0;

/**
 * \brief Hours, minutes, seconds expressed as a double
 */
double jhour = 0;

/**
 * \brief Minute part of time
 */
int minute = 0;

/**
 * \brief Month part of date
 */
int month = 0;

/**
 * \brief Name of the planet
 */
char name[AS_MAXCH];

/**
 * \brief Second part of time
 */
int second = 0;

/**
 * \brief Error message
 *
 * \sa swe_calc_ut()
 * \sa swe_fixstars_ut()
 */
char serr[AS_MAXCH];

/**
 * \brief Formatted date/time
 */
char strtime[AS_MAXCH];

/**
 * \brief Is the time set from the command line?
 */
int time_set = 0;

/**
 * \brief Year part of date
 */
int year = 0;

/*
* Function signatures
*/

void cleanUp (void);
void fail (char *);
void findDegree (int, double *);
void getGmtTime (void);
void getTime (time_t);
int handleCommandLine (int, char **);
void parseDate (char *);
void parsePlanets (int **, char *);
void parseTime (char *);
void printPlanet (double *);
void resizePlanets (int **, int);
void setEphemeris (char *);
void swap (int *, int *);
void printPlanets (double, int *, char *);

/**
 * \brief Main entry point of application
 *
 * This function is the main function where the program begins execution.
 *
 * \param argc Number of arguments passed in the command line
 * \param argv Array of arguments passed in the command line
 * \return int
*/
int
main (int argc, char **argv)
{
	int ret = 0;
	int i = 0;
	double x[6];
	double julday = 0;

	if (!handleCommandLine (argc, argv)) {
		cleanUp ();
		exit (EXIT_SUCCESS);
	}
	puts ("<?xml version=\"1.0\" encoding=\"UTF-8\" ?>");
	puts ("<data>");
	getGmtTime ();
	printf ("<timestamp>%u</timestamp>\n", (unsigned int) gmttime);
	julday = swe_julday (year, month, day, jhour, SE_GREG_CAL);
	if (bitflags & SPEED) {
		flags |= SEFLG_SPEED;
	}
	printPlanets (julday, planets, "planet");
	printPlanets (julday, fictitious_planets, "fictitiousPlanet");
	printPlanets (julday, stars, "star");
	printPlanets (julday, asteroids, "asteroid");
	puts ("</data>");
	cleanUp ();
	return EXIT_SUCCESS;
}

/**
 * \brief Gets the GMT date and time
 *
 * This function gets the GMT date and time.
 *
 * If #date_set is set, then the date is retrieved from #year, #month, and
 * #day.
 *
 * If #time_set is set, then the time is retrieved from #hour, #minute, and
 * #second.
 *
 * If neither #date_set nor #time_set is set to 1, then this function
 * retrieves the current GMT time.
 *
 * \return void
 */
void
getGmtTime (void)
{
	char *tz = NULL;
	char tzenv[50];

	time (&gmttime);	/* Get current time */
	tm = gmtime (&gmttime);	/* Convert to tm struct */

	/*
	 *      If date or time were set
	 */
	if (date_set || time_set) {

		/*
		 * Specifically, if date was set
		 */
		if (date_set) {
			tm->tm_year = year - 1900;	/* Set year to that specified on the command line */
			tm->tm_mon = month - 1;	/* Set month to that specified on the command line */
			tm->tm_mday = day;	/* Change day to that specified on the command line */
		}

		/*
		 * As long as either date or time were set, time should be
		 * changed
		 */
		tm->tm_hour = hour;	/* Change hour to that specified on the command line */
		tm->tm_min = minute;	/* Change minute to that specified on the command line */
		tm->tm_sec = second;	/* Change second to that specified on the command line */
		tz = getenv ("TZ");	/* Get timezone */
		putenv ("TZ=UTC0");	/* Change timezone to UTC */
		gmttime = mktime (tm);	/* Set timestamp to the time set in the tm struct */

		/*
		 * If a timezone was previously set
		 */
		if (tz) {

			/*
			 * Reset the timezone
			 */
			strcpy (tzenv, "TZ");
			strcat (tzenv, "=");
			strcat (tzenv, tz);
			putenv (tzenv);
		} else {

			/*
			 * Unset the timezone
			 */
			putenv ("TZ");
		}
	} else {
		year = tm->tm_year + 1900;
		month = tm->tm_mon + 1;
		day = tm->tm_mday;
		hour = tm->tm_hour;
		minute = tm->tm_min;
		second = tm->tm_sec;
	}
	jhour = hour + minute / 60 + second / 3600;
}

/**
 * \brief Handle the command line arguments
 *
 * This function uses getopt_long() to parse the command line arguments passed
 * to this program.
 *
 * \param argc Number of command line arguments
 * \param argv Array of command line arguments
 * \return int
 */
int
handleCommandLine (int argc, char **argv)
{
	struct option options[] = {
		{"help", no_argument, NULL, 'h'},
		{"version", no_argument, NULL, 'v'},
		{"date", required_argument, NULL, 'd'},
		{"time", required_argument, NULL, 't'},
		{"ephemeris-path", required_argument, NULL, 'D'},
		{"ephemeris", required_argument, NULL, 'e'},
		{"jpl_file", required_argument, NULL, 'j'},
		{"planets", required_argument, NULL, 'p'},
		{"fictitious-planets", required_argument, NULL, 'f'},
		{"stars", required_argument, NULL, 's'},
		{"asteroids", required_argument, NULL, 'a'},
		{"longitude", required_argument, NULL, 'l'},
		{"latitude", required_argument, NULL, 'L'},
		{"speed", required_argument, NULL, 'S'},
		{"find-degree", required_argument, NULL, 'F'},
		{0, 0, 0, 0}
	};
	int index;
	int c;

	while (1) {
		c = getopt_long (argc,
				 argv, "hvd:t:D:e:j:p:f:s:a:lLSF:", options,
				 &index);
		if (c == -1) {

			/*
			 * No more options
			 *
			 * Break out of loop
			 */
			break;
		}

		/*
		 * Check which argument we're dealing with
		 */
		switch (c) {
		 case 'h':

			 /*
			  * Help
			  */
			 printf
			  ("Astrology - Retrieve data about celestial bodies - ");
			 printf ("Version %s\n", VERSION);
			 puts ("Usage:");
			 printf ("%s {-h | -v} ", argv[0]);
			 return 0;
		 case 'v':

			 /*
			  * Version
			  */
			 printf ("Astrology\nVersion %s", VERSION);
			 return 0;
		 case 'd':

			 /*
			  * User-specified date
			  */
			 parseDate (optarg);
			 break;
		 case 't':

			 /*
			  * User-specified time
			  */
			 parseTime (optarg);
			 break;
		 case 'D':

			 /*
			  * Set ephemeris path
			  */
			 swe_set_ephe_path (optarg);
			 break;
		 case 'e':

			 /*
			  * Set type of ephemeris to use
			  */
			 setEphemeris (optarg);
			 break;
		 case 'j':

			 /*
			  * Location of JPL ephemeris
			  */
			 swe_set_jpl_file (optarg);
			 break;
		 case 'p':

			 /*
			  * Major planets
			  */
			 parsePlanets (&planets, optarg);
			 break;
		 case 'f':

			 /*
			  * Fictitious planets
			  */
			 parsePlanets (&fictitious_planets, optarg);
			 break;
		 case 's':

			 /*
			  * Stars
			  */
			 parsePlanets (&stars, optarg);
			 break;
		 case 'a':

			 /*
			  * Asteroids
			  */
			 parsePlanets (&asteroids, optarg);
			 break;
		 case 'l':

			 /*
			  * User wants longitude for all planets
			  */
			 bitflags |= LONGITUDE;
			 break;
		 case 'L':

			 /*
			  * User wants latitude for all planets
			  */
			 bitflags |= LATITUDE;
			 break;
		 case 'S':

			 /*
			  * User wants speed for all planets
			  */
			 bitflags |= SPEED;
			 break;
		 case 'F':

			 /*
			  * User wants to find when planet passes through given degree
			  */

			 /*
			  * Parse argument for valid double, giving the degree
			  * for which to search
			  */
			 if (!sscanf (optarg, "%lf", &find_degree)) {
  • 2 Contributors
  • 2 Replies
  • 268 Views
  • 7 Hours Discussion Span
  • Latest Post Latest Post by Nholdamek
Member Avatar for Salem

Probably because the memset on line 721 is trashing memory.
Whenever you have a <= and an array, it's a pretty sure bet you really meant <, and that you've just stepped off the end of the array.

Plus, it's all so unnecessary as well.
What you should have had is just one of these in place of the memset

if (i == size) {
			size += 5;
			resizePlanets (pplanets, size);
		}
		*(*pplanets + i) = END_OF_PLANETS;

> *pplanets = (int *) malloc (size * 4);
1. Don't cast malloc in C
http://c-faq.com/malloc/mallocnocast.html
2. Don't use literals for sizes - who said sizeof(int) was 4 ?
Do something like *pplanets = malloc (size * sizeof(int)); Or better yet, use this pattern, which is guaranteed to be correct no matter what type of pointer p is p = malloc ( n * sizeof *p ); which in your case results in *pplanets = malloc (size * sizeof **pplanets); > *pplanets = (int *) realloc (*pplanets, size * 4);
Classic realloc mis-use bug.
If realloc fails, it does NOT free the old memory.
But you just trashed your only pointer.
Do something like this

void *temp = realloc( *pplanets, size * sizeof **pplanets );
if ( temp != NULL ) {
  *pplanets = temp;
} else {
  // do something else, we ran out of memory
}

> *a ^= *b ^= *a ^= *b;
It's so cute - but totally wrong.
http://c-faq.com/expr/xorswapexpr.html
Just try swapping a variable with itself and sit back and enjoy the fun.
The more obvious (and correct) int temp = *a ; *a = *b ; *b = temp; is far more likely to be spotted as a swap by a compiler, and treated as such if it is capable of doing something special.

Member Avatar for Nholdamek

Hi,

Ah, thank you for those pointers (no pun intended, lol).

That's very interesting that one is not supposed to cast malloc.

concerning the memset, yeah that's just easier to set the end to END_OF_PLANETS, and so I've done that. I forget my reasoning for the <=, but obviously it was faulty.

concerning swap, didn't know that wasn't defined behavior. Thank you for that tip as well.

I'm still a little rusty in C (programmed in it a few years ago, but then went to other languages for a while), so yeah I'm still relearning.

This is working beautifully now, by the way. Thank you.

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.