Make ao_log_data re-entrant as it is used for both sensor and GPS logs

Because ao_log_data is called from two different threads, failing to
make it re-entrant would cause the 'log' pointer parameter to get
overwritten if another thread asked to log data while the eeprom was
busy writing out a block.

This would cause the second thread to re-writing data from the first
thread's address, but without re-checksumming the data as the checksum
is computed before the log mutex is taken.

The bug can be seen by log blocks with invalid checksums.

Here's what happens with the ao_gps_tracking_report and ao_log threads:

  ao_gps_tracking_report		ao_log

   Writes a bunch of records
   *blocks* in the eeprom flush
					sets ao_log_data 'log' to global 'log'
					computes checksum for 'log' block
					*blocks* on ao_log_mutex
   Wakes up
   sets ao_log_data 'log' to 'gps_log'
   writes remaining records
   'gps_log' is left with svid = 0
   *blocks* on ao_gps_tracking_data
					writes data, reading from
					the current ao_log_data 'log'
					pointer which points at 'gps_log'

Making ao_log_data re-entrant fixes this by ensuring that the 'ao_log'
thread has its own copy of the ao_log_data 'log' parameter.

I made this function take an __xdata restricted pointer so that it
could be passed in the dptr register instead of needing to go on the stack.

Signed-off-by: Keith Packard <keithp@keithp.com>

diff --git a/src/ao.h b/src/ao.h
index dfff8a8dc253f5d531466480c112140bd160de7b..5dd756dac48e21671d1f66f0615cfc2abfdce8af 100644 (file)
--- a/src/ao.h
+++ b/src/ao.h
@@ -562,7 +562,7 @@ struct ao_log_record {
 
 /* Write a record to the eeprom log */
 void
-ao_log_data(struct ao_log_record *log);
+ao_log_data(__xdata struct ao_log_record *log) __reentrant;
 
 /* Flush the log */
 void

diff --git a/src/ao_log.c b/src/ao_log.c
index 44ce90e0a943c88eb734e8d0eb52518198ffbedd..d550d40850cde2a8eef8b71c8d4828f7ea49306c 100644 (file)
--- a/src/ao_log.c
+++ b/src/ao_log.c
@@ -23,7 +23,7 @@ static __xdata uint8_t        ao_log_running;
 static __xdata uint8_t ao_log_mutex;
 
 static uint8_t
-ao_log_csum(uint8_t *b)
+ao_log_csum(__xdata uint8_t *b) __reentrant
 {
        uint8_t sum = 0x5a;
        uint8_t i;
@@ -34,11 +34,11 @@ ao_log_csum(uint8_t *b)
 }
 
 void
-ao_log_data(struct ao_log_record *log)
+ao_log_data(__xdata struct ao_log_record *log) __reentrant
 {
        /* set checksum */
        log->csum = 0;
-       log->csum = ao_log_csum((uint8_t *) log);
+       log->csum = ao_log_csum((__xdata uint8_t *) log);
        ao_mutex_get(&ao_log_mutex); {
                if (ao_log_running) {
                        ao_ee_write(ao_log_current_pos,