#190 ophelp -X doesn't display event's unit mask type

closed-fixed
None
5
2011-08-12
2010-08-26
No

oprofile-0.9.6-6.fc13.i686

It would be nice if "ophelp -X" displayed the unit mask type (mandatory, exclusive, bitmask) as an attribute in the <unit_masks> tag.

Discussion

  • Maynard Johnson

    Maynard Johnson - 2010-08-27

    Can you submit a patch? I'd be happy to review it.

     
  • Roland Grunberg

    Roland Grunberg - 2010-08-27

    Hi, the patch is was the attached file "unit_mask_type.patch".

    It's small enough that I could have probably posted it here, so sorry for the confusion.
    (hope this shows up correctly)

    diff -ru oprofile-0.9.6/libop/op_xml_events.c oprofile-0.9.6-new/libop/op_xml_events.c
    --- oprofile-0.9.6/libop/op_xml_events.c 2009-11-24 10:25:17.000000000 -0500
    +++ oprofile-0.9.6-new/libop/op_xml_events.c 2010-08-26 14:30:26.000000000 -0400
    @@ -80,6 +80,19 @@
    open_xml_element(HELP_UNIT_MASKS, 1, buffer, MAX_BUFFER);
    init_xml_int_attr(HELP_DEFAULT_MASK, event->unit->default_mask,
    buffer, MAX_BUFFER);
    + char um_type[10];
    + switch (event->unit->unit_type_mask){
    + case utm_bitmask:
    + strncpy(um_type, "bitmask", sizeof(um_type));
    + break;
    + case utm_exclusive:
    + strncpy(um_type, "exclusive", sizeof(um_type));
    + break;
    + case utm_mandatory:
    + strncpy(um_type, "mandatory", sizeof(um_type));
    + break;
    + }
    + init_xml_str_attr(HELP_UNIT_MASKS_TYPE, um_type, buffer, MAX_BUFFER);
    close_xml_element(NONE, 1, buffer, MAX_BUFFER);
    for (i = 0; i < event->unit->num; i++) {
    open_xml_element(HELP_UNIT_MASK, 1, buffer, MAX_BUFFER);
    diff -ru oprofile-0.9.6/libop/op_xml_out.c oprofile-0.9.6-new/libop/op_xml_out.c
    --- oprofile-0.9.6/libop/op_xml_out.c 2009-11-24 10:25:17.000000000 -0500
    +++ oprofile-0.9.6-new/libop/op_xml_out.c 2010-08-26 12:21:23.000000000 -0400
    @@ -80,6 +80,7 @@
    "ext",
    "unit_masks",
    "default",
    + "type",
    "unit_mask",
    "mask",
    "desc"
    diff -ru oprofile-0.9.6/libop/op_xml_out.h oprofile-0.9.6-new/libop/op_xml_out.h
    --- oprofile-0.9.6/libop/op_xml_out.h 2009-11-24 10:25:17.000000000 -0500
    +++ oprofile-0.9.6-new/libop/op_xml_out.h 2010-08-26 12:19:53.000000000 -0400
    @@ -54,6 +54,7 @@
    HELP_EXT,
    HELP_UNIT_MASKS,
    HELP_DEFAULT_MASK,
    + HELP_UNIT_MASKS_TYPE,
    HELP_UNIT_MASK,
    HELP_UNIT_MASK_VALUE,
    HELP_UNIT_MASK_DESC

     
  • Roland Grunberg

    Roland Grunberg - 2010-09-05

    Resubmitted patch as attachment to conform with contribution guidelines.

     
  • Maynard Johnson

    Maynard Johnson - 2010-09-07
    • assigned_to: nobody --> maynardj
     
  • Maynard Johnson

    Maynard Johnson - 2010-09-07

    Thanks for the patch submission. A few comments:
    1. The patch gets a compile error when applied to current CVS (minor one line change needed).
    2. I suggest changing the new unit mask attribute name from "type" to something else (e.g., "category"), since "type" is a reserved word in xml schema language. It may be OK to do, but it could tend to be confusing to some.
    3. The doc/ophelp.xsd should be updated to include the new optional attribute.

    Regarding comment #3, your patch motivated me to check our current ophelp schema doc and the ophelp XML instances our code creates using an XML validator, and unfortunately, I found some problems. I'll be posting a patch to fix those problems, which will include fixes to doc/ophelp.xsd, so please wait until that patch is committed to our CVS before you re-spin your patch to address my above comments. Oh, and when you do submit a new patch, could you also attach an example XML instance that I can run through the XML validator (the one I was using can be found at http://tools.decisionsoft.com/schemaValidate/\).

    Thanks.

     
  • Maynard Johnson

    Maynard Johnson - 2010-09-08

    Roland, I just posted a patch to the oprofile-list that fixes some problems in ophelp.xsd and the generated XML output. Please pull the latest oprofile CVS, apply that patch, and then respin your patch, addressing my aforementioned review comments.

     
  • Roland Grunberg

    Roland Grunberg - 2010-09-30

    Patch is posted, and applied on top of the one mentioned. xml file included as well.

     
  • Maynard Johnson

    Maynard Johnson - 2010-09-30

    Roland, your patch looks good, with one caveat. I think, by rights, we should bump the schema version to "1.1" since the new attribute being added is required.

    As you know, your patch is dependent on the patch that I posted to the list (mentioned below) to fix other unrelated schema issues with ophelp.xsd. I've not been able to get Richard Purdie's attention to review that patch. But as a user of ophelp's XML output, your review would be more than satisfactory. So please find that original posting of mine (Sept 8, subject "[Patch] Fix ophelp schema validation problems and xml generation error") and give it a thumbs up. Please also indicate that you are a consumer of ophelp XML output and mention that this patch is a pre-req to a patch you submitted for this bug. Once that's done, I'll commit both that patch and yours. Thanks.

     
  • Roland Grunberg

    Roland Grunberg - 2010-10-01
     Patch, Signed-off-by: Roland Grunberg roland.grunberg@gmail.com
    
     
  • Roland Grunberg

    Roland Grunberg - 2010-10-01

    Minor change to patch. (bump up schema version to 1.1)

     
  • Maynard Johnson

    Maynard Johnson - 2010-10-01

    Updated patch looks fine. Thanks.

     
  • Maynard Johnson

    Maynard Johnson - 2010-10-15

    Roland, I'd like to close out this bug and get your patch committed. But as I said in my September 30 comment to this bug . . .
    "your patch is dependent on the patch that I posted to the
    list (mentioned below) to fix other unrelated schema issues with
    ophelp.xsd. I've not been able to get Richard Purdie's attention to review
    that patch. But as a user of ophelp's XML output, your review would be
    more than satisfactory. So please find that original posting of mine (Sept
    8, subject "[Patch] Fix ophelp schema validation problems and xml
    generation error") and give it a thumbs up. Please also indicate that you
    are a consumer of ophelp XML output and mention that this patch is a
    pre-req to a patch you submitted for this bug. Once that's done, I'll
    commit both that patch and yours."

    Can you send your review comments to that patch to the oprofile-list please? Thanks.

     
  • Maynard Johnson

    Maynard Johnson - 2011-01-23

    The patch for this bug was committed to oprofile CVS on Oct 15, 2010. I neglected to update this bugzilla with that information until now. Changed "Resolution" field to "Fixed".

     
  • Maynard Johnson

    Maynard Johnson - 2011-01-23
    • status: open --> open-fixed
     
  • Suravee Suthikulpanit

    • status: open-fixed --> closed-fixed
     

Log in to post a comment.