Created attachment 135945 [details] [review]
Patch to fix bug

Hmm, might be simpler to just fix the docs ?

Also, I'm dubious what 

 * If the command line contains any escape characters defined inside the
 * storage specification, they will be expanded.

is referring to.


Ebassi ?

Hhm. You can't just fix the docs to say it doesn't duplicate because people will have written code that expects the behaviour according to the published API.

Any such code will crash when it frees the non-duplicated string, so I doubt there is too much affected code out there...

Reasonable point. Emmanuele, what would you like me to do?

since the application information is stored inside the RecentInfo and it's guaranteed to be valid until the RecentInfo is valid, then copying should be superfluous. at least, that was (as far as I remember) the line of reasoning I followed - I probably just forgot to update the documentation.

for that, and for Matthias comment, I'd say that the documentation should be fixed to say that "the app_exec string is owned by the RecentInfo structure and should not be modified or freed".

as a side node: hopefully I'll be able to finish the GIO integration, and we'll have a gtk_recent_info_get_g_app_info() returning a GAppInfo instead -- which should make gtk_recent_info_get_app_info() redundant (and possibly deprecated).

And you, Emmanuele, would be even more of a super-hero than usual. I'll work on a patch to do the other way, and uhm, fix my code. :-)

Created attachment 136355 [details] [review]
Fix the documentation in gtk_recent_info_get_application_info()

The documentation for the function says that the app_exec string
should be freed, but we return a pointer to the internal string
without duplicating it. Since the app_exec string is valid as long
as the GtkRecentInfo is valid the documentation should be fixed
and the out argument should be constified.

Fixes bug:

  Bug 584832 – Duplicate the exec string returned by
               gtk_recent_info_get_application_info

Looks good to me