Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FFI structs sample possible dangling pointer #145

Open
doubleday opened this issue Jun 6, 2022 · 3 comments
Open

FFI structs sample possible dangling pointer #145

doubleday opened this issue Jun 6, 2022 · 3 comments
Assignees
Labels

Comments

@doubleday
Copy link

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.

@mahesh-hegde
Copy link
Contributor

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.

@domesticmouse
Copy link
Member

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

@mit-mit mit-mit assigned dcharkes and unassigned mit-mit Jun 12, 2022
@dcharkes
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants