A little abstraction to have abstract data content bound to a type.
I would prefer the use of a goto and only one eina_stringshare_del in this function.
This is really a hack. Why are you not just returning: eina_list_iterator_new(possibilities).
Maybe it would be best to do something about this :-)
There is no close done on eina side. It should be properly documented. Also, from an API point of view, this isn't great as their is both a risk of leaking the fd and their could be a race condition in reopening the file instead of using a fd. Considering my comment above, it would actually be best to just have an Eina_File API.
This isn't great. Having a minimum support for conversion to and from STRING, BLOB, FILE would make things more usable.
The callbacks hash should be destroyed here.
I do not understand why it is a double pointer here.
I think you should choose either Eina_Content or Eina_Abstract_Content.
You could introduce easily an: EAPI Eina_File *eina_content_as_mmap(Eina_Content *content); that could be quite useful and work in all case (Even when the content is not in a file, you can still map an Eina_Slice to a virtual Eina_File).
Is it on purpose that it is not an Eina_Slice *?
I think register should be at the end of the function name.
This is a good example of the problem of the API as it is leaking the fd and actually double opening the file.
I dont want to publish Eina_Content_Conversion_Node as a struct. Which would mess up the iterator. (Pointer to a Pointer of a String)
Just replacing the last character ? (I am somewhat against resizing the whole thing, thats a little bit of a too big overhead). Or simply return NULL...?
I forgot the close ... :)
The API is not meant to return a Eina_File. This is just there to have a path fast at the hand that you can continue to use. (Like _tempfile_new in efl_ui_selection_manager.c). If someone wants to open the file again from the same process as this API is called, then he is doing something weird, he can just read the slice.
I went a lot back and forth with this. My problem is, i would only allow STRING conversion if its a utf8 string, which would only be a few cases, BLOB is the problem of "we don't know what it is" so i cannot think of a type for it. And File to a text/plain path would be possible, but there is the question of why you would do that.
If something shows up in future that we should have it, we still can add it, but i dont want to provide a swiss army knife that is not used at all :/
I forgot *value = NULL at the end of the function. I want it to be cleaned up because this here is freeing the content.
As I said before, the leaking fd is a error I made, the reason this here is double opening the file is that i actually want to test if the content does match the slice, which is nothing you will ever do as API user ... :)
Some more review.
Please use EINA_REFCOUNT.
Hum, this is not the only case we have for this. Maybe a cleaner solution would be to introduce a new iterator that take an iterator and an offset to return the pointer at that offset. Something like: eina_iterator_offset_pointer_new(Eina_Iterator *source, int offset);. You would use it here like: it = eina_iterator_offset_pointer_new(eina_list_iterator_new(possibilities), offsetof(Eina_Content_Convertion_Node, to));.
I think returning NULL in this case is acceptable. It means error to the caller anyway.
I see. I am ok with the close this way.
Just answer these questions so I can update the docs, thx!
Shouldn't the parameter be called content?
I'm not following this. Does eina_value_pset copy the content?
This is equivalent to eina_value_content_new but returns an obj instead of a ptr, right?
Except for the minor cosmetic of the .h file name not matching the function name, I am good with this. You might want to add a _unregister function as some of the register could be done in a module and disappear at any point.