multiple definition error when referenced by c++

[Sibada]

• VIC version 5.0.1 February 1, 2017
• gcc 4.9.3; g++ 4.9.3
• MinGW of RBuildTools 3.3

Feature Requests

I try to write an c++ extension of VIC to make it could be called by R easier. I found that some global variables were definited in the head files, including char vic_run_ref_str[MAXSTRING]; at line 95 of file vic/vic_run/include/vic_def.h, FILE *LOG_DEST; at line 61 of file vic/vic_run/include/vic_log.h, and double (*funcd)(double z, ... at line 177 of file vic/vic_run/include/vic_run.h. It causes multiple definition error when I let several cpp files include the vic_run.h. It seems to solved when I take them into a c file and place extern to their original definitions. Might it be proper?

[jhamman]

@Sibada - thank you for the report.

To be honest, we don't have much experience with C++. In plain C, we protect includes with

#ifndef VIC_RUN_H
#define VIC_RUN_H
...
#endif

Does this not work with C++?

Before we make any changes to VIC to support C++, we'd want to get Travis setup to also test with a C++ compiler. I know @mschnorb and @jameshiebert also have interest in using VIC with C++ so its not a terrible idea to explore this with VIC.5.

---

extension of VIC to make it could be called by R easier

I recognize there may be some interest here as I've heard about this from others as well. Perhaps it would be worth opening a separate issue to discuss how that would work.

[jameshiebert]

I'm not a C++ expert either, but IIRC we moved to C++ on VIC in our fork off of VIC 4.0.7 without much fuss using g++. I'd experiment with your compiler flags before you start ripping up code.

[Sibada]

@jhamman Yeah, it seems that C++ do not support placing global parameters in the head file even the #ifndef et al. were used, and I found that many other C and C++ developers suggest to place global parameters in a source file and used extern to define them in the head files.

[jhamman]

@Sibada - I think I understand. It may be instructive to see which changes you made to support using VIC with a C++ compiler. Would it be possible for you to open a PR with just those changes? Even better if you can enable a travis build with a C++ compiler too.

[Sibada]

@jhamman Those revises are applied on the master branch, might I should revise them on the develop branch and open the PR? My revises includes add an extern to the h files:

// vic/vic_run/include/vic_def.h
extern size_t NR;       /**< array index for force struct that indicates
                             the model step avarage or sum */
extern size_t NF;       /**< array index loop counter limit for force
                             struct that indicates the SNOW_STEP values */
extern char   vic_run_ref_str[MAXSTRING];

// vic/vic_run/include/vic_run.h
extern double (*funcd)(double z, double es, double Wind, double AirDens, double ZO,
                double EactAir, double F, double hsalt, double phi_r,
                double ushear,
                double Zrh);

// vic/vic_run/include/vic_log.h
extern FILE *LOG_DEST;

And move the definitions to a c file:

// my_R_package/src/global.c
size_t              NR;
size_t              NF;

FILE *LOG_DEST;
char vic_run_ref_str[MAXSTRING];

double (*funcd)(double z, double es, double Wind, double AirDens, double ZO,
        double EactAir, double F, double hsalt, double phi_r,
        double ushear,
        double Zrh);

I also found that some C std library functions have no implements in windows platform, whatever VC or MinGW are used, including strptime, backtrace and backtrace_symbols. I though in the R package some macro defines should be used like:

// vic/drivers/shared_all/src/vic_log.c
void
print_trace(void)
{
#ifndef _WIN32
    void  *array[50];
    size_t size;
    char **strings;
    size_t i;

    size = backtrace(array, 50);
    strings = backtrace_symbols(array, size);

    /* Note: we are ignoring the first and last stack frame
       the first is this function, the last is the linker, neither are
       particularly useful */
    fprintf(LOG_DEST,
            "---------------------------------------------------------------------------\n");
    fprintf(LOG_DEST, "Traceback (most recent call last):\n");
    for (i = size - 2; i > 0; i--) {
        fprintf(LOG_DEST, "%s\n", strings[i]);
    }

    free(strings);
#endif
}

And the strptime also needs a specified implement.

[jhamman]

@Sibada - why don't you rebase your branch (from master to develop) and issue a pull request so we can see if this actually works. If this is all that is required, I think making these changes is possible, provided we can test them appropriately.