Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify finding decoder logic #93

Closed
wants to merge 1 commit into from

Conversation

brchiu
Copy link
Contributor

@brchiu brchiu commented May 16, 2016

Taking 'FileCreationTimeStamp' of sample TAP3 as example, the corresponding ASN.1 expression of tap3.asn1 are :

FileCreationTimeStamp ::= [APPLICATION 108] DateTimeLong

DateTimeLong ::= SEQUENCE 
{
    localTimeStamp     LocalTimeStamp OPTIONAL, -- *m.m.
    utcTimeOffset      UtcTimeOffset  OPTIONAL
}

Part of generated FileCreationTimeStamp.c is :

...
asn_dec_rval_t
FileCreationTimeStamp_decode_ber(asn_codec_ctx_t *opt_codec_ctx, asn_TYPE_descriptor_t *td,
        void **structure, const void *bufptr, size_t size, int tag_mode) {
    FileCreationTimeStamp_1_inherit_TYPE_descriptor(td);
    return td->ber_decoder(opt_codec_ctx, td, structure, bufptr, size, tag_mode);
}

static void
FileCreationTimeStamp_1_inherit_TYPE_descriptor(asn_TYPE_descriptor_t *td) {
    td->free_struct    = asn_DEF_DateTimeLong.free_struct;
    td->print_struct   = asn_DEF_DateTimeLong.print_struct;
    td->check_constraints = asn_DEF_DateTimeLong.check_constraints;
    td->ber_decoder    = asn_DEF_DateTimeLong.ber_decoder;
    td->der_encoder    = asn_DEF_DateTimeLong.der_encoder;
    td->xer_decoder    = asn_DEF_DateTimeLong.xer_decoder;
    td->xer_encoder    = asn_DEF_DateTimeLong.xer_encoder;
    td->uper_decoder   = asn_DEF_DateTimeLong.uper_decoder;
    td->uper_encoder   = asn_DEF_DateTimeLong.uper_encoder;
    if(!td->per_constraints)
        td->per_constraints = asn_DEF_DateTimeLong.per_constraints;
    td->elements       = asn_DEF_DateTimeLong.elements;
    td->elements_count = asn_DEF_DateTimeLong.elements_count;
    td->specifics      = asn_DEF_DateTimeLong.specifics;
}

asn_TYPE_descriptor_t asn_DEF_FileCreationTimeStamp = {
    "FileCreationTimeStamp",
    "FileCreationTimeStamp",
    FileCreationTimeStamp_free,
    FileCreationTimeStamp_print,
    FileCreationTimeStamp_constraint,
    FileCreationTimeStamp_decode_ber,
    FileCreationTimeStamp_encode_der,
    FileCreationTimeStamp_decode_xer,
    FileCreationTimeStamp_encode_xer,
    0, 0,   /* No PER support, use "-gen-PER" to enable */
    0,  /* Use generic outmost tag fetcher */
    asn_DEF_FileCreationTimeStamp_tags_1,
    sizeof(asn_DEF_FileCreationTimeStamp_tags_1)
        /sizeof(asn_DEF_FileCreationTimeStamp_tags_1[0]) - 1, /* 1 */
    asn_DEF_FileCreationTimeStamp_tags_1,   /* Same as above */
    sizeof(asn_DEF_FileCreationTimeStamp_tags_1)
        /sizeof(asn_DEF_FileCreationTimeStamp_tags_1[0]), /* 2 */
    0,  /* No PER visible constraints */
    0, 0,   /* Defined elsewhere */
    0   /* No specifics */
};

And part of generated DateTimeLong.c

asn_TYPE_descriptor_t asn_DEF_DateTimeLong = {
    "DateTimeLong",
    "DateTimeLong",
    SEQUENCE_free,
    SEQUENCE_print,
    SEQUENCE_constraint,
    SEQUENCE_decode_ber,
    SEQUENCE_encode_der,
    SEQUENCE_decode_xer,
    SEQUENCE_encode_xer,
    0, 0,   /* No PER support, use "-gen-PER" to enable */
    0,  /* Use generic outmost tag fetcher */
    asn_DEF_DateTimeLong_tags_1,
    sizeof(asn_DEF_DateTimeLong_tags_1)
        /sizeof(asn_DEF_DateTimeLong_tags_1[0]), /* 1 */
    asn_DEF_DateTimeLong_tags_1,    /* Same as above */
    sizeof(asn_DEF_DateTimeLong_tags_1)
        /sizeof(asn_DEF_DateTimeLong_tags_1[0]), /* 1 */
    0,  /* No PER visible constraints */
    asn_MBR_DateTimeLong_1,
    2,  /* Elements count */
    &asn_SPC_DateTimeLong_specs_1   /* Additional specs */
};

With eye-inspection, we can see when invoking ber_decoder of asn_DEF_FileCreationTimeStamp,
function SEQUENCE_decode_ber is called eventually. And the address of asn_SPC_DateTimeLong_specs_1 is assigned to asn_DEF_FileCreationTimeStamp's member variable 'specifics'.

Moreover, asn_MBR_DateTimeLong_1 is used for accessing child elements when proceeding the process of decoding each child element of DateTimeLong.

So after some tweak of asn1c_C.c and other files, we can eliminate the indirect access of codec function and generate the final data structure for asn_DEF_FileCreationTimeStamp directly. Thus we can remove the excessive helper functions which occupy a lot of code space.

New type descriptor in FileCreationTimeStamp.c

asn_TYPE_descriptor_t asn_DEF_FileCreationTimeStamp = {
    "FileCreationTimeStamp",
    "FileCreationTimeStamp",
    SEQUENCE_free,
    SEQUENCE_print,
    FileCreationTimeStamp_constraint,
    SEQUENCE_decode_ber,
    SEQUENCE_encode_der,
    SEQUENCE_decode_xer,
    SEQUENCE_encode_xer,
    0, 0,   /* No PER support, use "-gen-PER" to enable */
    0,  /* Use generic outmost tag fetcher */
    asn_DEF_FileCreationTimeStamp_tags_1,
    sizeof(asn_DEF_FileCreationTimeStamp_tags_1)
        /sizeof(asn_DEF_FileCreationTimeStamp_tags_1[0]) - 1, /* 1 */
    asn_DEF_FileCreationTimeStamp_tags_1,   /* Same as above */
    sizeof(asn_DEF_FileCreationTimeStamp_tags_1)
        /sizeof(asn_DEF_FileCreationTimeStamp_tags_1[0]), /* 2 */
    0,  /* No PER visible constraints */
    asn_MBR_DateTimeLong_1,
    2,  /* Elements count */
    &asn_SPC_DateTimeLong_specs_1   /* Additional specs */
};

There are some more possible improvement to this pull request :

  1. check_constraints function has not been manipulated as above due to, for some types, constraint checks are really needed. For those check_constraints functions have similar content, it can be removed as well. For those check_constraints functions really do check, either leave it as current or change to a data driven function.
    We might need to modify asn1c_C.c and perhaps asn1c_constraint.c to achieve these.
  2. After codec functions are all mapped to primitive types, then we can extract them into a codec function array and use a type variable to access them. Then we can reduce excessive pointers to codec function to a single numeric (or enumerated) variable. This can further reduce code size.
  3. After codec functions are extracted, it become easier for us to conditionally remove codec functions that we don't need. For example, if we only want PER coder and decoder, we can conditionally remove XER/BER/DER codec. This is especially useful for RRC whose message are PER oriented.

Notice :

  1. I have tested the pull request by 'make check' using examples under RRC, TAP3, LDAP. As for PKIX1 and MHEG5 and 'make distcheck', they failed the same place as 0.9.28. And ULP and MEGACO has not been tested yet due to no complete asn1 file and no test data available.
  2. I am not quite acquainted with the internal of asn1c nor with ASN.1 specification. All modification are done by comparison and simple logic inference. The modified code can work with minimum level but not in good quality in terms of code structure or flow. Any revise advice is welcomed.

@brchiu brchiu force-pushed the simplify_finding_decoder_logic branch from d7ca449 to 584914f Compare May 20, 2016 04:49
@brchiu
Copy link
Contributor Author

brchiu commented May 20, 2016

Item 1 is implemented and add to this pull request.

@brchiu brchiu force-pushed the simplify_finding_decoder_logic branch from 584914f to c70fefd Compare November 9, 2016 13:14
@brchiu brchiu force-pushed the simplify_finding_decoder_logic branch from c70fefd to 01bfcfc Compare November 18, 2016 12:26
@brchiu brchiu force-pushed the simplify_finding_decoder_logic branch from 01bfcfc to 7c08aea Compare March 16, 2017 14:28
@brchiu
Copy link
Contributor Author

brchiu commented Mar 16, 2017

Fix 'make check' failures.

@brchiu brchiu force-pushed the simplify_finding_decoder_logic branch from 7c08aea to f5d328d Compare March 17, 2017 06:30
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 77.538% when pulling 4ab5d5e on brchiu:simplify_finding_decoder_logic into 0eca8c3 on vlm:master.

@coveralls
Copy link

coveralls commented Mar 28, 2017

Coverage Status

Coverage increased (+0.03%) to 77.884% when pulling 0372693 on brchiu:simplify_finding_decoder_logic into 0eca8c3 on vlm:master.

@brchiu brchiu force-pushed the simplify_finding_decoder_logic branch from 0372693 to 65e0e8a Compare May 19, 2017 02:29
@coveralls
Copy link

coveralls commented May 19, 2017

Coverage Status

Coverage decreased (-0.01%) to 77.894% when pulling 65e0e8a on brchiu:simplify_finding_decoder_logic into 4021e4b on vlm:master.

@@ -4825,7 +4825,7 @@ check-per:
done; done; fi
@if test -f sample-DL-DCCH-Message-1.per ; then \
for f in sample-*-[1-9].per; do \
pdu=`echo $$f | sed -E -e "s/sample-([A-Za-z-]+)-[0-9].*/\1/"`; \
pdu=`echo $$f | sed -E -e "s/sample-([A-Za-z-]+)-[0-9].*//"`; \
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional?

@vlm
Copy link
Owner

vlm commented Aug 2, 2017

Merged, thank you!

(Though see my comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants