Bug 9088 - [PATCH] Freed frame ../source3/libsmb/clilist.c:934, expected ../source3/client/clitar.c:821
Summary: [PATCH] Freed frame ../source3/libsmb/clilist.c:934, expected ../source3/clie...
Alias: None
Product: Samba 4.0
Classification: Unclassified
Component: Other (show other bugs)
Version: 4.0 beta4
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: samba4-qa@samba.org
Depends on:
Reported: 2012-08-11 18:46 UTC by Sal Gonzalez
Modified: 2012-09-24 03:37 UTC (History)
1 user (show)

See Also:

Patch to properly free ctx in do_tar. (2.88 KB, application/octet-stream)
2012-08-11 18:46 UTC, Sal Gonzalez
no flags Details
Patch to add a simple tarmode test. (2.95 KB, patch)
2012-08-11 18:47 UTC, Sal Gonzalez
no flags Details
0002-Add-smbclient-tarmode-test.patch (3.25 KB, patch)
2012-08-12 18:06 UTC, Sal Gonzalez
no flags Details
0003-Fix-smbclient-tarmode-panic-on-connecting-to-Windows.patch (3.02 KB, patch)
2012-08-12 18:07 UTC, Sal Gonzalez
vl: review+
metze: review+
0004-Do-not-use-tmp-for-file-storage.patch (5.34 KB, patch)
2012-08-12 18:08 UTC, Sal Gonzalez
no flags Details
0001-Fix-copy-paste-error-in-test-usage-string.patch (849 bytes, patch)
2012-08-12 18:09 UTC, Sal Gonzalez
no flags Details
Updated tests (5.40 KB, patch)
2012-08-13 18:28 UTC, Sal Gonzalez
no flags Details
make_test.log (10.86 KB, text/plain)
2012-08-13 18:30 UTC, Sal Gonzalez
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sal Gonzalez 2012-08-11 18:46:37 UTC
Created attachment 7756 [details]
Patch to properly free ctx in do_tar.

Function do_tar tallocs() memory for ctx, but never frees it before returning.  WHen running tarmode against a windows 2000 client I hit this when the first test 'Freed frame ../source3/libsmb/clilist.c:934, expected ../source3/client/clitar.c:821' evaluates to true.  Doesn't happen against WinXP clients.
Comment 1 Sal Gonzalez 2012-08-11 18:47:45 UTC
Created attachment 7757 [details]
Patch to add a simple tarmode test.

As per abartlett, I have attempted to create a test for tarmmode... I'm not too confident if this has any value.
Comment 2 Sal Gonzalez 2012-08-11 18:48:24 UTC
This is against 4.0.0beta5, but it wasn't available as a version.
Comment 3 Sal Gonzalez 2012-08-11 18:49:46 UTC
Whoops, the test is (strequal(finfo->name,"..") || strequal(finfo->name,".")).
Comment 4 Andrew Bartlett 2012-08-11 21:42:23 UTC
This looks good.  However we just need to avoid using /tmp, and instead use the selftest prefix.  Please pass in $PREFIX like the previous example in tests.py

Comment 5 Andrew Bartlett 2012-08-11 21:50:07 UTC
The other thing that would be good is to actually check the contents of the tar file against the original directories, or the remote server (using $LOCAL_PATH).
Comment 6 Sal Gonzalez 2012-08-11 23:40:19 UTC
I'll give it a shot.  Would it be worthwile to also do the test in 'reverse' as well, ie. tar -x; mget?
Comment 7 Sal Gonzalez 2012-08-12 18:06:51 UTC
Created attachment 7758 [details]

Same patch, exported with format-patch
Comment 8 Sal Gonzalez 2012-08-12 18:07:27 UTC
Created attachment 7759 [details]

Same patch, exported with format-patch
Comment 9 Sal Gonzalez 2012-08-12 18:08:51 UTC
Created attachment 7760 [details]

Utilize LOCAL_PATH & PREFIX, verify data integrity, test both creation and extraction
Comment 10 Sal Gonzalez 2012-08-12 18:09:48 UTC
Created attachment 7761 [details]

Quick typo fix I stumbled upon.  Doesn't really belong in this bug, per-se.
Comment 11 Sal Gonzalez 2012-08-12 18:11:04 UTC
I did not actuall *test* the changes to the test.  Is there a way to selectively run a test, or must all of them run?
Comment 12 Andrew Bartlett 2012-08-12 21:57:17 UTC
Run 'make test TESTS=tarmode' to test just tests with tarmode in the name.
Comment 13 Sal Gonzalez 2012-08-13 18:28:30 UTC
Created attachment 7762 [details]
Updated tests

Updated (still not working) test
Comment 14 Sal Gonzalez 2012-08-13 18:29:53 UTC
I can't seem to get this test to work... When I run a make test, I am getting NT_STATUS_LOGON_FAILURE.
Comment 15 Sal Gonzalez 2012-08-13 18:30:42 UTC
Created attachment 7763 [details]

Output from make test TESTS="tarmode"
Comment 16 Jeremy Allison 2012-08-13 19:32:56 UTC
Comment on attachment 7759 [details]

Obviously correct, and applies cleanly to 3.6.next also.

Please review and I'll get this re-assigned to Karolin for inclusion.


Comment 17 Andrew Bartlett 2012-08-13 23:34:38 UTC
This patch isn't needed except for developer builds on master, as the panic is the check that rusty introduced to check that we were using talloc_stackframe() correctly.  

The remaining patches are now on their way to master, after I worked out the issues with the testsuite.
Comment 18 Jeremy Allison 2012-08-13 23:43:52 UTC
Sure, but it is an obvious mistake, with a very clear fix - and it exists in 3.6.x.

Comment 19 Stefan Metzmacher 2012-08-14 06:11:59 UTC
Comment on attachment 7759 [details]

Looks goodb
Comment 20 Stefan Metzmacher 2012-08-14 06:12:34 UTC
Karolin, please pick for the next release
Comment 21 Karolin Seeger 2012-08-15 18:04:38 UTC
Pushed to v3-6-test.
Closing out bug report.

Thanks a lot!