On Fri, May 16, 2014 at 08:51:36AM +0200, Jakub Hrozek wrote:
On Thu, May 15, 2014 at 09:12:22PM +0300, Nikolai Kondrashov wrote:
> On 04/17/2014 01:13 PM, Lukas Slebodnik wrote:
> >On (17/04/14 10:44), Jakub Hrozek wrote:
> >>On Thu, Apr 17, 2014 at 11:06:16AM +0300, Nikolai Kondrashov wrote:
> >>>On 04/17/2014 02:29 AM, Lukas Slebodnik wrote:
> >>>>It does not make sense to argue.
> >>>>
> >>>>test-io was not ideal unit test and adding another
workaroud(fix/hack) is not
> >>>>good. I could sleep, but ...
> >>>>
> >>>>Patch is attached.
> >>>
> >>>Thanks, Lukas. This fixes my problem.
> >>
> >>I have one more request -- don't use strdup and free, but rather create
> >>a test context in the setup function and just free the test context in
> >>the teardown function. I know the current code works, but it's not the
> >>best example for future developers (who tend to look at unit tests for
> >>examples on how to use code).
> >>
> >>Conversely, get_random_filepath should accept a TALLOC_CTX as the first
> >>parameter.
> >
> >I didn't use talloc for this small test, because I did not want to add
> >new libraries in Makefile.am (test_io_LDADD)
> >
> >>
> >>You can also consider creating a destructor for the contents of struct
> >>dir_state, but here I think the teardown function is semantically close
> >>enough to a destructor.
>
> Jakub, what do you think about Lukas' response? Do you think his patch still
> needs the changes you requested, or you can merge it?
>
> Nick
Sorry about letting this thread stall.
I'm not exactly thrilled about not using talloc, but the test works fine
and doesn't leak any memory.
ACK.
Pushed to master: f333ca01311000475db0fbd059243d05f9a90e96