On 02/12/2017 03:51 PM, Noriko Hosoi wrote: > > https://pagure.io/389-ds-base/issue/49121 > > https://pagure.io/389-ds-base/issue/raw/files/d857ff4919940bcebeae8707748... > > These odd » characters are shown at some part of the patch. I wonder > what causes this display issue... And non-ascii characters are not > printed correctly although they are in the "View Raw" mode. Please > see the utf8str.txt file. > > 277 @@ -1499,30 +1506,36 @@ entry2str_internal_size_attrlist( const > Slapi_Attr *attrlist, int entry2str_ctrl > 280 » » /* Count the space required for the present and deleted values */ > 281 -» » elen+= entry2str_internal_size_valueset(a->a_type, > &a->a_present_values, > 282 -» » » » » » » » » » » » entry2str_ctrl, attribute_state, > 283 -» » » » » » » » » » » » VALUE_PRESENT); 305 +» » elen += entry2str_internal_size_valueset(a, a->a_type, > &a->a_present_values, > 306 +» » entry2str_ctrl, attribute_state, VALUE_PRESENT); > > > Note: The test build was blessed by the bug reporter. Thanks to William for his reviews. I've update the patch based on his suggestion. I also have 2 lib389 patches (attached to this email). Is lib389 still in fedorahosted? I cloned pagure.io/lib389.git, but I found it empty... 1) could you please review the patches?

2) if they look fine, is it ok to push them to fedorahosted git repository?

On Sun, 2017-02-12 at 17:42 -0800, Noriko Hosoi wrote: > On 02/12/2017 03:51 PM, Noriko Hosoi wrote: >> https://pagure.io/389-ds-base/issue/49121 >> >> https://pagure.io/389-ds-base/issue/raw/files/d857ff4919940bcebeae8707748... >> >> These odd » characters are shown at some part of the patch. I wonder >> what causes this display issue... And non-ascii characters are not >> printed correctly although they are in the "View Raw" mode. Please >> see the utf8str.txt file. >> >> 277 @@ -1499,30 +1506,36 @@ entry2str_internal_size_attrlist( const >> Slapi_Attr *attrlist, int entry2str_ctrl >> 280 » » /* Count the space required for the present and deleted values */ >> 281 -» » elen+= entry2str_internal_size_valueset(a->a_type, >> &a->a_present_values, >> 282 -» » » » » » » » » » » » entry2str_ctrl, attribute_state, >> 283 -» » » » » » » » » » » » VALUE_PRESENT); 305 +» » elen += entry2str_internal_size_valueset(a, a->a_type, >> &a->a_present_values, >> 306 +» » entry2str_ctrl, attribute_state, VALUE_PRESENT); >> >> >> Note: The test build was blessed by the bug reporter. > Thanks to William for his reviews. I've update the patch based on his > suggestion. > > I also have 2 lib389 patches (attached to this email). Is lib389 still > in fedorahosted? I cloned pagure.io/lib389.git, but I found it empty... > > 1) could you please review the patches? I'm happy with the dbscan change I don't use the valgrind wrapper myself (it should disable transparently when ASAN is enabled).

I'm not sure about the check for sbin dir though, because it shouldn't matter what we use. Is there a reason you need to check for root with just those dirs? Is it related to where the DS instance is from (prefix build vs rpm)

> 2) if they look fine, is it ok to push them to fedorahosted git repository? I would ask Mark this, he knows the most about the situation. _______________________________________________ 389-devel mailing list -- 389-devel(a)lists.fedoraproject.org To unsubscribe send an email to 389-devel-leave(a)lists.fedoraproject.org

On (13/02/17 08:38), Noriko Hosoi wrote: > On 02/12/2017 06:14 PM, William Brown wrote: >> On Sun, 2017-02-12 at 17:42 -0800, Noriko Hosoi wrote: >>> On 02/12/2017 03:51 PM, Noriko Hosoi wrote: >>>> https://pagure.io/389-ds-base/issue/49121 >>>> >>>> https://pagure.io/389-ds-base/issue/raw/files/d857ff4919940bcebeae8707748... >>>> >>>> These odd » characters are shown at some part of the patch. I wonder >>>> what causes this display issue... And non-ascii characters are not >>>> printed correctly although they are in the "View Raw" mode. Please >>>> see the utf8str.txt file. >>>> >>>> 277 @@ -1499,30 +1506,36 @@ entry2str_internal_size_attrlist( const >>>> Slapi_Attr *attrlist, int entry2str_ctrl >>>> 280 » » /* Count the space required for the present and deleted values */ >>>> 281 -» » elen+= entry2str_internal_size_valueset(a->a_type, >>>> &a->a_present_values, >>>> 282 -» » » » » » » » » » » » entry2str_ctrl, attribute_state, >>>> 283 -» » » » » » » » » » » » VALUE_PRESENT); 305 +» » elen += entry2str_internal_size_valueset(a, a->a_type, >>>> &a->a_present_values, >>>> 306 +» » entry2str_ctrl, attribute_state, VALUE_PRESENT); >>>> >>>> >>>> Note: The test build was blessed by the bug reporter. >>> Thanks to William for his reviews. I've update the patch based on his >>> suggestion. >>> >>> I also have 2 lib389 patches (attached to this email). Is lib389 still >>> in fedorahosted? I cloned pagure.io/lib389.git, but I found it empty... >>> >>> 1) could you please review the patches? >> I'm happy with the dbscan change >> >> I don't use the valgrind wrapper myself (it should disable transparently >> when ASAN is enabled). > Are there any chance to support both valgrind and ASAN? Well, I'm not at the > position to insist anything any more here :p, but they are just tools and > supporting both of them is not a bad idea, is it? FYI: My experienceis that you should use either ASAN or valgrind. They do not work well togeteher. That's the same for other sanitizers.

ASAN is faster but valgrind should catch more bugs. Of course it can change in future :-) my 2 cents LS _______________________________________________ 389-devel mailing list -- 389-devel(a)lists.fedoraproject.org To unsubscribe send an email to 389-devel-leave(a)lists.fedoraproject.org

On Mon, 2017-02-13 at 09:31 -0800, Noriko Hosoi wrote: > On 02/13/2017 08:51 AM, Lukas Slebodnik wrote: > > On (13/02/17 08:38), Noriko Hosoi wrote: > >> On 02/12/2017 06:14 PM, William Brown wrote: > >>> On Sun, 2017-02-12 at 17:42 -0800, Noriko Hosoi wrote: > >>>> On 02/12/2017 03:51 PM, Noriko Hosoi wrote: > >>>>> https://pagure.io/389-ds-base/issue/49121 > >>>>> > >>>>> https://pagure.io/389-ds-base/issue/raw/files/d857ff4919940bcebeae8707748... > >>>>> > >>>>> These odd » characters are shown at some part of the patch. I wonder > >>>>> what causes this display issue... And non-ascii characters are not > >>>>> printed correctly although they are in the "View Raw" mode. Please > >>>>> see the utf8str.txt file. > >>>>> > >>>>> 277 @@ -1499,30 +1506,36 @@ entry2str_internal_size_attrlist( const > >>>>> Slapi_Attr *attrlist, int entry2str_ctrl > >>>>> 280 » » /* Count the space required for the present and deleted values */ > >>>>> 281 -» » elen+= entry2str_internal_size_valueset(a->a_type, > >>>>> &a->a_present_values, > >>>>> 282 -» » » » » » » » » » » » entry2str_ctrl, attribute_state, > >>>>> 283 -» » » » » » » » » » » » VALUE_PRESENT); 305 +» » elen += entry2str_internal_size_valueset(a, a->a_type, > >>>>> &a->a_present_values, > >>>>> 306 +» » entry2str_ctrl, attribute_state, VALUE_PRESENT); > >>>>> > >>>>> > >>>>> Note: The test build was blessed by the bug reporter. > >>>> Thanks to William for his reviews. I've update the patch based on his > >>>> suggestion. > >>>> > >>>> I also have 2 lib389 patches (attached to this email). Is lib389 still > >>>> in fedorahosted? I cloned pagure.io/lib389.git, but I found it empty... > >>>> > >>>> 1) could you please review the patches? > >>> I'm happy with the dbscan change > >>> > >>> I don't use the valgrind wrapper myself (it should disable transparently > >>> when ASAN is enabled). > >> Are there any chance to support both valgrind and ASAN? Well, I'm not at the > >> position to insist anything any more here :p, but they are just tools and > >> supporting both of them is not a bad idea, is it? > > FYI: > > My experienceis that you should use either ASAN or valgrind. > > They do not work well togeteher. That's the same for other sanitizers. They *can not* be operated together. They both try to intercept malloc, and they break each other. > I see... Thanks, Lukas! > > The current test scripts call valgrind APIs only if it's built without > ASAN enabled as follows. That is, if ASAN is enabled in the CI, the > valgrind check won't be executed. Probably, we could move this > "has_asan" check to the inside of the valgrind APIs and keep the APIs in > lib389? I thought I had already done that? It really only affects me anyway. > > *if not topology_m2.ms["master1"].has_asan():* > results_file = valgrind_get_results_file(topology_m2.ms["master1"]) > > Another question is, when the CI test enables ASAN, is this test case -- > checking the valgrind result and determining the test case succeeded or > failed -- still valid? Or should we rewrite it based upon the ASAN > syntax (if any) later? We can use both in the way here, I don't see a reason to change. In the ASAN mode, the DS exits with a fail code, which crashes the test, even if the valgrind wrapper "passes". > > Thanks, > --noriko > > > > ASAN is faster but valgrind should catch more bugs. > > Of course it can change in future :-) I disagree, but that's what we are entitled to do. ASAN is easier to setup (compile only, no need to hack scripts),

triggers errors faster (lib389 picks up errors immediately, no need to parse output)

gives better traces (shows where the leak occured, where it was allocated, and can even show memory addresses for you to set watch points on to debug) and it's faster.

https://pagure.io/389-ds-base/issue/49121 https://pagure.io/389-ds-base/issue/raw/files/d857ff4919940bcebeae8707748... These odd » characters are shown at some part of the patch. I wonder what causes this display issue... And non-ascii characters are not printed correctly although they are in the "View Raw" mode. Please see the utf8str.txt file.

277 @@ -1499,30 +1506,36 @@ entry2str_internal_size_attrlist( const Slapi_Attr *attrlist, int entry2str_ctrl 280 » » /* Count the space required for the present and deleted values */ 281 -» » elen+= entry2str_internal_size_valueset(a->a_type, &a->a_present_values, 282 -» » » » » » » » » » » » entry2str_ctrl, attribute_state, 283 -» » » » » » » » » » » » VALUE_PRESENT); 305 +» » elen += entry2str_internal_size_valueset(a, a->a_type, &a->a_present_values, 306 +» » entry2str_ctrl, attribute_state, VALUE_PRESENT); Note: The test build was blessed by the bug reporter. _______________________________________________ 389-devel mailing list -- 389-devel(a)lists.fedoraproject.org To unsubscribe send an email to 389-devel-leave(a)lists.fedoraproject.org