Native C string (char*) to java String

Technical Java
1

I’m trying to get the Java ZomB bindings working, and I seem to be having an issue with getting returned C strings (a const char* to be exact). I have this function:


private static final Function getstringfnc = NativeLibrary.getDefaultInstance().getFunction("ZomBDashboardGetString");
public String GetString(String name)
{
    int returnstring = getstringfnc.call1(Pointer.createStringBuffer(name));
    return Pointer.NativeUnsafeGetString(Address.fromPrimitive(returnstring));
}

and this C declaration:

extern "C"
{
    const char* ZomBDashboardGetString(const char* name);
}

and every time I call GetString(“value”), my program dies (but it stills says code on the DS). I can, however, pass strings to functions fine.
Java Source attached, C header and out file are on FIRST Forge (just use the latest build’s installer, and its in C:\Program Files\ZomB\Bindings)

src.zip (4.3 KB)


src.zip (4.3 KB)

2

I don’t see a killer bug, but there is one thing to clean up.

  1. Each call to Pointer.createStringBuffer() essentially “mallocs” a new C string buffer, so you typically need to free it after calling the C function (unless the C code stashes the value somewhere to use later). To fix this, create a local Pointer variable, say “ptr”, and call ptr.free() after the call out to C.

Also, I’m not sure what died, and what the symptoms are. Something on the cRIO or the driver station?

And I couldn’t find the C code for ZomBDashboardGetString(), etc. Is it in the svn somewhere?

Good luck!

3

I’m not sure about what died either (Netbeans won’t connect and send me print statements), but I do know that this code (in teleop code):

zombinstance.Send("taco", joystick.GetY());

works, and any controls hooked up to taco work, but this code

zombinstance.Send("taco", joystick.GetY());
zombinstance.Send("value", zombinstance.GetString("invalue"));

does not, and all controls are dead, which leads me to assume that it kills Java somehow (but the DS still says Enabled and code) (and If I only execute the GetString function if a button is pressed, it dies when the button is pressed)

The h file should be in C:\Program files\ZomB\Bindings\ (or Start>Programs>ZomB>Bindings) and the cpp file is compiled into ZomB.out, but you can see it in svn (\releasefiles\bindings\source), but I’ve attached both

ZDashboard.cpp (22.8 KB)
ZDashboard.h (8.23 KB)


ZDashboard.cpp (22.8 KB)
ZDashboard.h (8.23 KB)

4

It seems that you may be leaking a fair amount of memory. Remember that in C++ (unlike Java) you can’t just new something (like a string or array of char) and then forget about it and expect a garbage collector to clean up after you. You need to be more deliberate about your allocations. Such as:

char* System451::Communication::Dashboard::ZomBTCPSource::readBytes(int number)
{
    char* buf = new char[number+1];
    ...

Or:

void System451::Communication::Dashboard::ZomBTCPSource::sendreader()
{
    ...
    curvalues*new string(buf)] = const_cast<const char*>(vbuf);
    ...

On top of that, creating a new “string” allocates even more memory to copy the heap-allocated char] into and then leaks the char].

Any time you are done with a heap-allocated ("new"ed) variable, you should delete it. Such as here:

int System451::Communication::Dashboard::ZomBTCPSource::readByte()
{
    char r = this->readBytes(1)[0];
    ...

And technically when you new an array, you should use an array delete (“delete ] buf;”), though with a primitive type (char) I don’t think it makes a practical difference. Such as:

char* System451::Communication::Dashboard::ZomBTCPSource::readBytes(int number)
{
    ...
    delete buf2;
    ...

Also, you are using a map to store the values. Maps don’t allow you to add a value if its key is already present in the map. That means that if it is possible for you to get an updated value instead of always adding new values, then you need to delete the old value from the map before adding the new one. Otherwise you leak both the new name “string” and the old value char].

You are returning a C++ allocated string back to Java and depending on it sticking around. That’s a safe assumption I guess if you never delete anything, but you really should be deleting them when they change. The way this is typically done is to allocate a buffer in Java, pass that out to the “GetString” function, and strcpy the value into the Java string. This way Java is in control of the lifetime of that allocated memory. If you are worried about the string length, then you can split the C entry-point into 2 parts and request the length first and then allocate a buffer in Java and then get the string.

I wouldn’t be surprised if things were dying as a result of some of these memory allocation issues.

-Joe

5

I have been able to get labview to work fine with it, so i’m sure its on the Java side call, since it dies IMMEDIATELY, and I’ve gotten C++ and LabVIEW to stick around for at least a minute.

6

Have you tried using the stand-alone NetConsole client to monitor what’s happening at this time? You should be able to use both it and NetBeans at the same time (though probably not both on the same machine).

I’m guessing you are abbreviating here… I see no Send method that takes parameters… only an Add() that takes parameters, which you then follow up with a Send(). Do you get the “crash” or “death” or whatever if you just call GetString and then use it in a System.out.println or something?

If the DS still says there is code, then at least part of the JVM is still running a DS thread. Perhaps you are seeing a deadlock between your Java objects. Or maybe a task in C++ is crashing while holding a sync object that one of these calls needs. However, you don’t seem to be using the BlockingFunction JNA function object, so if you blocked outside of Java, the whole VM would hang and stop the DS thread in Java.

See if you can isolate the issue a bit more.

-Joe

7

Ok, i’ve narrowed it down a bit, and the crashing is due to Double.parseDouble("") throwing an exception because GetString is always returning “” (empty string)

public String GetString(String name)
    {
        Pointer ptr = Pointer.createStringBuffer(name);
        int retturn = getstringfnc.call1(ptr);
        Address adr = Address.fromPrimitive(retturn);
        String str = Pointer.NativeUnsafeGetString(adr);
        System.out.println("String:'"+str+"'");
        ptr.free();
        return str;
    }
    public double GetDouble(String name)
    {
        try
        {
            return Double.parseDouble(GetString(name));
        }
        catch(Throwable t) { }
        return 0;
    }

and

public void operatorControl() {
getWatchdog().setEnabled(false);
    while (isOperatorControl()) {

        if (z.CanSend()) {
            z.Add("taco", stick.getY());
            z.Add("val", z.GetDouble("ival"));
            z.Send();
        }
        Timer.delay(0.01);
    }
    }

produces

String:''
String:''
String:''
String:''
String:''
String:''
String:''
String:''
String:''
String:''
String:''
String:''
8

Have you ever seen the “NativeUnsafeGetString” actually work?

9

No, it just seemed like it should

10

I recommend that in both LabVIEW and Java, change the API such that you allocate a string in the client language, pass the pointer out to your API to be populated, then consume it in the client language. This should resolve your problem.

-Joe

11

changing the C sig to

void ZomBDashboardGetStringViaArg(const char* name, char* outValue)
{
    strcpy(outValue, ZomBDashboard::GetInstance().GetString(string(name)).c_str());
}

and the code in Get String to

        Pointer ptr = Pointer.createStringBuffer(name);//arg
        Pointer p = new Pointer(50);//50 should be enough for now
        getstringfnc.call2(ptr, p);//call
        String str = p.getString(0);//get our string
        System.out.println("String:'"+str+"'");

still prints out

String:‘’
String:‘’
String:‘’
String:‘’
String:‘’
String:‘’
String:‘’
String:‘’
String:‘’
String:‘’
String:‘’
String:‘’
String:‘’
String:‘’
String:‘’
String:‘’
String:‘’
String:‘’
String:‘’
String:‘’
…

12

Oh! Duh!
I was initializing it as
ZomBDashboard zomB = ZomBDashboard.getInstance(ZomBModes.TCP, “10.4.51.5”);
not as
ZomBDashboard zomB = ZomBDashboard.getInstance(ZomBModes.AllTCP, “10.4.51.5”);
It works now!