samples icon indicating copy to clipboard operation
samples copied to clipboard

FFI structs sample possible dangling pointer

Open doubleday opened this issue 2 years ago • 3 comments

Please ignore my ignorance but while I'm trying to learn more about ffi I studied the structs sample.

I just don't understand how freeing myHomeUtf8 does not leave a dangling pointer in the the native Place struct.

https://github.com/dart-lang/samples/blob/master/ffi/structs/structs_library/structs.c#L58

  struct Place create_place(char *name, double latitude, double longitude)
  {
    struct Place place;
    place.name = name;
    // ...

https://github.com/dart-lang/samples/blob/master/ffi/structs/structs.dart#L94

  final myHomeUtf8 = 'My Home'.toNativeUtf8();
  final createPlace =
      dylib.lookupFunction<CreatePlaceNative, CreatePlace>('create_place');
  final place = createPlace(myHomeUtf8, 42.0, 24.0);
  calloc.free(myHomeUtf8);
  final name = place.name.toDartString();

I understand that someone needs to free that memory and that in a sample memory management might not be the biggest concern. But accessing the name after freeing the memory seems especially odd to me.

Most likely I'm just missing something here.

doubleday avatar Jun 06 '22 13:06 doubleday

Yes apparently a small oversight. You might get an error like "UTF8 unexpected byte" while running it. You can just put calloc.free after converting name to dart string.

Also I think you can refer to samples folder and tests in SDK repo. There are more examples there. Samples here are relatively short.

mahesh-hegde avatar Jun 11 '22 16:06 mahesh-hegde

Hey @mit-mit can you drag in the right people to answer this question?

domesticmouse avatar Jun 12 '22 03:06 domesticmouse

Yes, this leaves a dangling pointer.

  • toNativeUtf8 allocates memory in C land.
  • the Pointer in Dart points to that memory
  • that pointer is passed as an argument to create_place
  • that pointer, still pointing to the same memory is stored in the struct, not the string itself, but the pointer.

Then, this struct is returned by value, which is a pattern prone to error.

In C++, one could use a C++ class instead, and have a destructor that frees the string.

In C, if the desire is to pass the struct around by value, I would inline the string by value.

struct Place
{
    char name[64];
    struct Coordinate coordinate;
};

struct Place create_place(char *name, double latitude, double longitude)
{
    struct Place place;
    strcpy(place.name, name);
    place.coordinate = create_coordinate(latitude, longitude);
    return place;
}

Note that now there is a max name length.

Alternatively, one could change the whole Place API to work on Place*. One would then malloc and free within the API, and provide a free(Place* place) function that frees both the place and the string.

On a higher level note, this seems to be a copy and paste error from other function calls in which a char* argument does not outlive the function call. (For example reverse.) In C, one always has to reason about lifetimes themselves.

dcharkes avatar Jun 13 '22 07:06 dcharkes