[SSSD] [PATCH 1/1] tests: Don't assume absolute test dir in test_io.c

Jakub Hrozek jhrozek at redhat.com
Fri May 16 06:51:36 UTC 2014


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.



More information about the sssd-devel mailing list