API for spectrum1d

I am wondering why this simple code shows out memory leaks with valgrind:


#include < gmt.h >
#include < string.h >
#include < math.h >

int main () {
  int i, m = 128;
  double sampling = 1.;
  void *API;
  struct GMT_DATASET *Din = NULL;
  struct GMT_DATASET *Dout = NULL;
  struct GMT_DATASEGMENT *S = NULL;
  char input[GMT_VF_LEN] = {""};
  char output[GMT_VF_LEN] = {""};
  uint64_t par[] = {1, 1, 512, 1};
  char command[2*GMT_VF_LEN + 64];

/* Initialize the GMT session */
API = GMT_Create_Session ("GMT_spectrum1d", 2, 0, NULL);
/* Create the data */
Din = GMT_Create_Data(API, GMT_IS_DATASET, GMT_IS_NONE, GMT_CONTAINER_AND_DATA, par, NULL, NULL, 0, -1, NULL);
S = Din->table[0]->segment[0];
for (i = 0; i < 512; i++) 
    S->data[0][i] = sin(2.*(double)i) + 2.*cos((double)i); // stupid data, just for testing
/* Open a virtual file to hold the data */
GMT_Open_VirtualFile(API, GMT_IS_DATASET, GMT_IS_NONE, GMT_IN, Din, input);
/* Open a virtual file to hold the result */
GMT_Open_VirtualFile(API, GMT_IS_DATASET, GMT_IS_NONE, GMT_OUT, NULL, output);
/* Create the parameters string */
sprintf(command, "%s -S%d -D%lf -W -N ->%s", input, 2*m, sampling, output);
/* Call the module */
GMT_Call_Module(API, "spectrum1d", GMT_MODULE_CMD, command);
/* Free input stuff */
GMT_Close_VirtualFile(API, input);
GMT_Destroy_Data(API, &Din);          // is the use of the address of Din correct here?
/* Get the spectrum in memory */
Dout = GMT_Read_VirtualFile(API, output);
/* Close output file */
GMT_Close_VirtualFile(API, output);
/* Print the result on stdout */
S = Dout->table[0]->segment[0];
for (i = 0; i < S->n_rows; i++) 
    printf("%f %f\n", S->data[0][i], S->data[1][i]);
if (S->n_rows != m)
    printf("%d rows (%d expected)\n", S->n_rows, m);
/* Free output */
GMT_Destroy_Data(API, &Dout);
/* Destroy session */
if (GMT_Destroy_Session (API)) return EXIT_FAILURE;
}

gcc testapi_spectrum1d.c -g $(gmt-config --cflags) $(gmt-config --libs) -lm -o testapi_spectrum1d

valgrind ./testapi_spectrum1d                                  
==5981== Memcheck, a memory error detector
==5981== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==5981== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright info
==5981== Command: ./testapi_spectrum1d
==5981== 
==5981== Invalid read of size 4
==5981==    at 0x4E9C254: gmtapi_import_dataset (in /usr/local/gmt/gmt-6.2.0/lib/libgmt.so.6.2.0)
==5981==    by 0x4EA848C: GMT_Read_Data (in /usr/local/gmt/gmt-6.2.0/lib/libgmt.so.6.2.0)
==5981==    by 0x51C9578: GMT_spectrum1d (in /usr/local/gmt/gmt-6.2.0/lib/libgmt.so.6.2.0)
==5981==    by 0x4E90DDD: GMT_Call_Module (in /usr/local/gmt/gmt-6.2.0/lib/libgmt.so.6.2.0)
==5981==    by 0x108C9E: main (testapi_spectrum1d.c:31)
==5981==  Address 0x1d390a38 is 56 bytes inside a block of size 80 free'd
==5981==    at 0x4C2CDDB: free (vg_replace_malloc.c:530)
==5981==    by 0x4E9C13F: gmtapi_import_dataset (in /usr/local/gmt/gmt-6.2.0/lib/libgmt.so.6.2.0)
==5981==    by 0x4EA848C: GMT_Read_Data (in /usr/local/gmt/gmt-6.2.0/lib/libgmt.so.6.2.0)
==5981==    by 0x51C9578: GMT_spectrum1d (in /usr/local/gmt/gmt-6.2.0/lib/libgmt.so.6.2.0)
==5981==    by 0x4E90DDD: GMT_Call_Module (in /usr/local/gmt/gmt-6.2.0/lib/libgmt.so.6.2.0)
==5981==    by 0x108C9E: main (testapi_spectrum1d.c:31)
==5981==  Block was alloc'd at
==5981==    at 0x4C2DBC5: calloc (vg_replace_malloc.c:711)
==5981==    by 0x4EF9192: gmt_memory_func (in /usr/local/gmt/gmt-6.2.0/lib/libgmt.so.6.2.0)
==5981==    by 0x4EEAF04: gmt_get_dataset (in /usr/local/gmt/gmt-6.2.0/lib/libgmt.so.6.2.0)
==5981==    by 0x4E9BDC6: gmtapi_import_dataset (in /usr/local/gmt/gmt-6.2.0/lib/libgmt.so.6.2.0)
==5981==    by 0x4EA848C: GMT_Read_Data (in /usr/local/gmt/gmt-6.2.0/lib/libgmt.so.6.2.0)
==5981==    by 0x51C9578: GMT_spectrum1d (in /usr/local/gmt/gmt-6.2.0/lib/libgmt.so.6.2.0)
==5981==    by 0x4E90DDD: GMT_Call_Module (in /usr/local/gmt/gmt-6.2.0/lib/libgmt.so.6.2.0)
==5981==    by 0x108C9E: main (testapi_spectrum1d.c:31)

The program returns the result, but when I repeat these instructions in a loop (which I need to do), it ends up crashing.
Perhaps spectrum1d does not expect the input data to be GMT_IS_DATASET but rather GMT_IS_VECTOR? But the doc tells that all modules wait for GMT_IS_DATASET as input, so I don’t know what to think… Wouldn’t there be a bug in gmt_api_import_dataset?

Thanks for reading (a bit long, I know), and helping!

Olivier

Thanks for the clear example, Olivier. I will reproduce the problem and have a look this afternoon. On first look I do not see you doing anything wrong - any module that takes a data set can take GMT_DATASET or MATRIX or VECTOR.

While it makes no difference, you should use GMT_IS_LINE instead of GMT_IS_NONE since spectral analysis implies a series.

Please use this line for the input virtual declaration:

GMT_Open_VirtualFile (API, GMT_IS_DATASET, GMT_IS_LINE, GMT_IN|GMT_IS_REFERENCE, Din, input);

and let me know if this has problems with valgrind too ( the default, GMT_IS_DUPLICATE gives me lots of memory-related warnings so I know there are issues there).

Hi Paul,

Using GMT_IS_REFERENCE, valgrind does not detect the problem anymore. Using GMT_LINE makes indeed no difference at all: memory problem with GMT_IS_DUPLICATE and no problem with GMT_IS_REFERENCE.
I am going to try that in my loop and will tell you if it is OK.

Works perfectly well this way.
If I get it right, I should always prefer using GMT_IS_REFERENCE, for it avoids duplicating my data in memory. I can’t event think of a use case for which I should not… Any idea?
On the output side, if I set my VirtualFile with GMT_OUT|GMT_IS_REFERENCE, does it mean that GMT_Read_VirtualFile returns the pointer to the result in the lib instead of a copy of the result? That seems to be preferable too, isn’t it?
Many thanks for your help.

I think normally you would always use GMT_IS_REFERENCE on input - GMT should override in those cases it does not work. On output, usually best to let GMT allocate memory and then you retrieve the pointer as you do now. Busy week for me but I will try to get back to this asap.

BTW, I believe I have fixed the memory issues in the API for the default (GMT_IS_DUPLICATE) case, so if you have time to update, rebuild, and test the original case again for any lingering issue it will be appreciated. It runs well on macOS now. PR

I confirm that you have fixed the memory issues: valgrind does not show any more error now.
Valgrind still shows a few memory leaks when the process ends (mainly some strings allocated by strdup within gmt_init_module which are not freed). The amount of memory is so small (304 bytes for my test case) that it is not really important, but I tell you just in case…