Could write_changes_to_disk be called twice?

[yli-cpr]

Just noticed this while reading the source code. When HAVE_LIBRADOS is true and HAVE_MMAP is false, write_changes_to_disk could be called twice?

#ifdef HAVE_LIBRADOS
    if (rrd_file->rados)
      write_changes_to_disk(&rrd, rrd_file, version);
#endif
#ifndef HAVE_MMAP
    if (write_changes_to_disk(&rrd, rrd_file, version) == -1) {
        goto err_free_structures;
    }
#endif

[oetiker]

oh ... indeed ... can you PR ?

[yli-cpr]

I would. But I really know nothing about rrdtool, and don't have a test environment either. I am not certain what a correct fix here. Will this be good?

#if defined(HAVE_LIBRADOS)
      ...
#elif not defined(HAVE_MMAP)
     ...
#endif 

[oetiker]

how about this:

#ifdef HAVE_LIBRADOS
    if (rrd_file->rados) {
       if (write_changes_to_disk(&rrd, rrd_file, version) == -1) {
          goto err_free_structures;
       }
    }
#ifndef HAVE_MMAP
    else
#endif
#endif
#ifndef HAVE_MMAP
    if (write_changes_to_disk(&rrd, rrd_file, version) == -1) {
        goto err_free_structures;
    }
#endif

[yli-cpr]

Pull request created: https://github.com/oetiker/rrdtool-1.x/pull/1059