GNOME Bugzilla – Bug 773731
Port CamelMedium to CamelNameValueArray
Last modified: 2016-11-01 11:43:55 UTC
This new CamelNameValueArray is really neat to use!
Created attachment 338844 [details] [review] [evolution] Port to new new CamelMedium API
Created attachment 338845 [details] [review] [EDS] The new CamelMedium API
Review of attachment 338844 [details] [review]: The patch looks good, it builds fine as well. Feel free to commit to the wip/camel-more-gobject branch ::: src/em-format/e-mail-part-headers.c @@ +93,1 @@ + camel_name_value_array_get (headers, ii, &header_name, &header_value); being over-paranoic, I would assign to both header_xxx variables NULL and check for non-NULL of both, before passing them to the g_ascii_strcasecmp(). I know it wasn't the case before.
Review of attachment 338845 [details] [review]: I found only the coding style issues, otherwise looks good. Similar to the evo part, please commit to the wip/camel-more-gobject branch. Also, I didn't notice it when reading the evolution patch, but it would be nice to check the return value of camel_name_value_array_get() there as well. ::: src/camel/camel-medium.h @@ +74,3 @@ const gchar *name); + CamelNameValueArray * + (*get_headers) (CamelMedium *medium); coding style: indent one more tab to the right @@ +90,3 @@ void camel_medium_remove_header (CamelMedium *medium, const gchar *name); +const gchar *camel_medium_get_header (CamelMedium *medium, coding style: missing tab before function name (beware of tabs on the right of the function name) @@ +93,2 @@ const gchar *name); +CamelNameValueArray * camel_medium_get_headers (CamelMedium *medium); coding style: the return type is too long, add the function name to the next line, but at the same indentation level as the previous functions (two tabs on the left).
This change breaks evolution-mapi, but do not worry, I'll adapt it.
What if we rename camel_medium_get_headers() to camel_medium_dup_headers()? Then it'll be clear that the returned pointer is not tight to the internal headers any more. See how the CamelMessageInfo accessors do it. I'd say it's a clear API, from which you understand the life time of the return value without much need to read the documentation.
Created attachment 338852 [details] [review] ema patch for evolution-mapi;
This is in wip/camel-more-gobject branches of respective products right now. (In reply to Milan Crha from comment #6) > What if we rename camel_medium_get_headers() to camel_medium_dup_headers()? And I did this one as well. I'm closing this bug report.