[Webkit-unassigned] [Bug 143872] [SOUP] Add initial implementation of NetworkProcess disk cache

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=143872

--- Comment #15 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #14)
> Comment on attachment 251154 [details]
> Another update
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=251154&action=review
> 
> >>> Source/WebKit2/NetworkProcess/cache/NetworkCacheDataSoup.cpp:41
> >>> +    : m_buffer(adoptGRef(soup_buffer_new(SOUP_MEMORY_COPY, data, size)))
> >> 
> >> We should probably discuss whether we want to use the WebKit memory allocator.
> > 
> > I'm not sure I get what you mean.
> 
> I'm simply suggesting that it might be worth it to use bmalloc here and then
> to free the memory properly by using soup_buffer_new_with_owner instead.

Ah, ok, I thought you were suggesting to replace soup buffer somehow.

> >>> Source/WebKit2/NetworkProcess/cache/NetworkCacheDataSoup.cpp:94
> >>> +    uint8_t* data = static_cast<uint8_t*>(g_malloc(size));
> >> 
> >> Ditto.
> > 
> > Soup uses g_free unconditionally, so when it takes the memory it needs to be allocated with g_malloc().
> 
> We would need to use soup_buffer_new_with_owner.

Got it.

> >>> Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:101
> >>> +    }
> >> 
> >> It is annoying that GFileError cannot be used here for the error code, because the enum starts at zero. Perhaps a comment here about that could be informative.
> > 
> > It can be used, but I didn't care about that yet, I don't even know if those errors are actually handled, or just used to check whether it failed or not.
> 
> Hrm. Isn't the first enum member of GFileError (G_FILE_ERROR_EXIST) defined
> to be 0?

My point is that I haven't cared about the actual error codes at this point because the caller use the error as a boolean (just to check whether it failed) or for the debug logs, so it doesn't really matter.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150424/952ed40d/attachment.html>