Merge PR99 and its fixes to support parsing Information Object and Information Object Set #154

brchiu · 2017-05-07T13:44:24Z

This is a collection of works :

Based on @zhanglei002 pull request 99.
A fix by @AuthenticEshkinKot and @ikarso, merged by @mouse07410 at
commit 2c8d36... of https://github.com/mouse07410/asn1c
to support parsing of Information Object and Information Object Set
A fix by @uri Blumenthal in asn1fix_derefv.c at :
commit ec0ade... of https://github.com/mouse07410/asn1c
to solve crash on asn1p_value_free().
My pull request 18 to @mouse07410's https://github.com/mouse07410/asn1c to solve
problems found during parsing ASN.1 modules of S1AP, RANAP and J2735-201603.
My pull request 22 to @mouse07410's https://github.com/mouse07410/asn1c to solve issue 147
and to solve the problem during parsing ASN.1 module of NBAP.
My pull request 23 to @mouse07410's https://github.com/mouse07410/asn1c to fix memory leakage
introduced in aforementioned commits.
Most code changes are the same as pull request 153 to this repository.
A fix of my bug in item 6 which result asn1c crash, fixed by @mouse07410.
My fix for another crash caused by item 6.
Add several test results for check-parsing.sh.
Apply @velichkov 's commit to @mouse07410's https://github.com/mouse07410/asn1c for 'Do not generate coverage data for the test C code' and also remove CODE_COVERAGE_CFLAGS in skeletons/Makefile.am
Temporarily remove make check in travis.yml to avoid Travis CI failure due to log larger than 4M bytes.

coveralls · 2017-05-07T13:48:08Z

Coverage decreased (-0.2%) to 77.745% when pulling 95317cd on brchiu:vlm_master_merge_pr99_mouse_2c8d366b_solve_prob_for_s1ap_nbap_dsrc into 4021e4b on vlm:master.

@zhanglei002

…formation Object Set This is a collection of works : 1. Based on @zhanglei002's pull request 99. 2. A fix by @AuthenticEshkinKot and merged by @mouse07410 at commit 2c8d366 of https://github.com/mouse07410/asn1c to support parsing of Information Object and Information Object Set 3. A fix by @uri Blumenthal in asn1fix_derefv.c at : commit ec0ade4 of https://github.com/mouse07410/asn1c to solve crash on asn1p_value_free(). 4. My pull request 18 to @mouse07410's https://github.com/mouse07410/asn1c to solve problems found during parsing ASN.1 modules of S1AP, RANAP and J2735-201603. 5. My pull request 22 to @mouse07410's https://github.com/mouse07410/asn1c to solve issue 147 and to solve the problem during parsing ASN.1 module of NBAP. 6. My pull request 23 to @mouse07410's https://github.com/mouse07410/asn1c to fix memory leakage introduced in aforementioned commits. Most code changes are the same as pull request 153 to this repository. 7. A fix of my bug in item 6 which result asn1c crash, fixed by @mouse07410.

coveralls · 2017-05-07T14:24:13Z

Coverage decreased (-0.2%) to 77.746% when pulling 80fd306 on brchiu:vlm_master_merge_pr99_mouse_2c8d366b_solve_prob_for_s1ap_nbap_dsrc into 4021e4b on vlm:master.

coveralls · 2017-05-08T04:20:17Z

Coverage decreased (-0.1%) to 77.769% when pulling 2d6b8c0 on brchiu:vlm_master_merge_pr99_mouse_2c8d366b_solve_prob_for_s1ap_nbap_dsrc into 4021e4b on vlm:master.

Excerpt from commit log : Do not generate coverage data for the test C code When compiling tests remove the CODE_COVERAGE_CFLAGS flags from CFLAGS and also add CFLAGS = $(filter-out $(CODE_COVERAGE_CFLAGS), @CFLAGS@) to 'skeleton/Makefile.am' to avoid compilation error in vlm's repository.

coveralls · 2017-05-08T09:14:41Z

Coverage decreased (-6.2%) to 71.66% when pulling c46137b on brchiu:vlm_master_merge_pr99_mouse_2c8d366b_solve_prob_for_s1ap_nbap_dsrc into 4021e4b on vlm:master.

Travis CI failed due to weird reason : The log length has exceeded the limit of 4 MB \ (this usually means that the test suite is raising \ the same exception over and over). The job has been terminated A temporary solution is removing `make check` and only perform `make distcheck` to reduce log size.

vlm · 2017-05-08T13:38:13Z

Thank you! Looking into it.

vlm · 2017-05-08T13:38:30Z

.travis.yml

@@ -9,7 +9,6 @@ script:
    - autoreconf -iv
    - ./configure --enable-Werror --enable-code-coverage
    - make -j8
-    - make check


What's the problem that prompted this?

Travis CI reports log larger than 4M bytes thus abort

The log length has exceeded the limit of 4 MB (this usually means that the test suite is raising the same exception over and over). The job has been terminated

This is just a workaround to reduce log size. I am not able to figure out why.
Please feel free to rollback it at your discretion.

Perhaps something like make check 2>&1 | tail - 1000 would work best?

ya, good idea.

vlm · 2017-05-08T13:39:12Z

libasn1fix/asn1fix_cws.c

+			);
+	} else {
+		WARNING("asn1c only be able to parse TypeFieldSpec and FixedTypeValueFieldSpec. Failed when parsing %s at line %d\n", p, arg->expr->_lineno);
+		free(p);  /* bad idea freeing object you refer to later! */


Not clear I understand the comment. Why?

According to 11.7 of X.681, Setting's BNF is :

Setting ::= Type | Value | ValueSet | Object | ObjectSet

Currently only Type (A1TC_CLASSFIELD_TFS) and Value (A1TC_CLASSFIELD_FTVFS) are implemented.
Sorry for my bad English wording, please feel free to change it.

Please refer to pr 22 to mouse07410's repository

oh.

About the comment : /* bad idea freeing object you refer to later! */.
The story is : when I sent pr 23 to @mouse07410's repository, i.e. item 6 in my description, I made a mistake that put free(p) before WARNING(...) which cause asn1c crash since the warning message use content pointed by pointer p.

@mouse07410 add the above comment to remind readers. And I merged the code segment into here without removing it.

vlm · 2017-05-08T13:41:07Z

libasn1compiler/asn1c_misc.c

 	/*
 	 * Make sure we have this amount of storage.
 	 */
 	if(storage_size <= size) {
-		free(storage);
+		if(storage) free(storage);


There's a parallel trend in this code base to remove if(foo) free(foo), since free is NULL-safe. Is there a particular reason to reintroduce if(storage) here?

Ya, you are right, because foo is of type static char * so it is initialized as 0.
It is not necessary to check against null.

Please feel free to remove it.

vlm · 2017-05-08T13:41:33Z

libasn1compiler/asn1c_misc.c

+		 * Add MODULE name to resolve clash
+		 */
+		if(expr->_mark & TM_NAMECLASH) {
+			size += strlen(expr->module->ModuleName) + 2;


When there are type names clash in different modules detected in asn1f_check_duplicate() of asn1fix.c, I add module name as prefix to the type name when making identifier for them :

M1 DEFINITIONS ::= BEGIN V ::= INTEGER END M2 DEFINITIONS ::= BEGIN V ::= OCTET STRING END

The constructed type name becomes M1_V and M2_V instead V and V.
So space for one underscore character '_' and one null terminate character \0 equals 2.

By the way, I think TM_NAMECLASH is not a good name (due to my poor English and limited vocabulary) and I am not sure whether expr->_mark is a good place to hold the information what this expr's name has duplication.
Please feel free to improve them at your solely discretion. Thanks.

Got it, thank you.

brchiu · 2017-05-08T13:44:39Z

Hi, I am reinstalling my laptop, might take some time before I can reply. Please stay tuned.

brchiu · 2017-05-08T15:14:26Z

By the way, this pull request only concentrates on parse and fix phases when dealing information object/information object set. But it does not address on the compilation phase. There seems no data structures or files generated for information object and/or information object set.

Hope someone with deeper knowledge for ASN.1 and asn1c code can continue working on it.

vlm · 2017-05-08T16:02:44Z

@brchiu, Thank you for improving the parsing. Indeed compilation phase is separate and I'll be able to help with that. Your patch is important and relevant on its own.

vlm · 2017-06-27T15:22:51Z

Merged, thank you very much!

These are unmerged lines of code that I forgot adding in pull request vlm#154.

…aints PER: fix encoding and decoding of OPENTYPE with size constraints

brchiu force-pushed the vlm_master_merge_pr99_mouse_2c8d366b_solve_prob_for_s1ap_nbap_dsrc branch from 95317cd to 80fd306 Compare May 7, 2017 14:21

Add more test result for check-parsing.sh

2d6b8c0

brchiu force-pushed the vlm_master_merge_pr99_mouse_2c8d366b_solve_prob_for_s1ap_nbap_dsrc branch from 2383076 to 2d6b8c0 Compare May 8, 2017 07:30

vlm reviewed May 8, 2017

View reviewed changes

vlm merged commit 993eca8 into vlm:master Jun 27, 2017

brchiu referenced this pull request Jun 27, 2017

new contributor

6da0272

brchiu added a commit to brchiu/asn1c that referenced this pull request Jun 28, 2017

Fix compile SAE J2735_201603DA.ASN issue

94f0b64

These are unmerged lines of code that I forgot adding in pull request vlm#154.

zhanglei002 mentioned this pull request Jun 29, 2017

add support for parsing Information Object and Information Object Set. #99

Closed

brchiu mentioned this pull request Jun 29, 2017

Fix compile SAE J2735_201603DA.ASN issue #168

Merged

brchiu referenced this pull request Aug 5, 2017

skeleton for J2735

e29e612

brchiu mentioned this pull request Aug 11, 2017

Basic types failed to be handled in object definition #147

Closed

brchiu deleted the vlm_master_merge_pr99_mouse_2c8d366b_solve_prob_for_s1ap_nbap_dsrc branch October 22, 2017 13:25

attina pushed a commit to attina/asn1c that referenced this pull request Dec 26, 2023

Merge pull request vlm#154 from CedricRouxEurecom/opentype-per-constr…

df2b288

…aints PER: fix encoding and decoding of OPENTYPE with size constraints

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge PR99 and its fixes to support parsing Information Object and Information Object Set #154

Merge PR99 and its fixes to support parsing Information Object and Information Object Set #154

brchiu commented May 7, 2017 •

edited

coveralls commented May 7, 2017 •

edited

coveralls commented May 7, 2017 •

edited

coveralls commented May 8, 2017 •

edited

coveralls commented May 8, 2017 •

edited

vlm commented May 8, 2017

vlm May 8, 2017

brchiu May 8, 2017

vlm May 8, 2017

brchiu May 8, 2017

vlm May 8, 2017

brchiu May 8, 2017 •

edited

brchiu May 8, 2017

brchiu May 10, 2017

vlm May 8, 2017

brchiu May 8, 2017

vlm May 8, 2017

brchiu May 8, 2017

vlm May 8, 2017

brchiu commented May 8, 2017

brchiu commented May 8, 2017

vlm commented May 8, 2017

vlm commented Jun 27, 2017

Merge PR99 and its fixes to support parsing Information Object and Information Object Set #154

Merge PR99 and its fixes to support parsing Information Object and Information Object Set #154

Conversation

brchiu commented May 7, 2017 • edited

coveralls commented May 7, 2017 • edited

coveralls commented May 7, 2017 • edited

coveralls commented May 8, 2017 • edited

coveralls commented May 8, 2017 • edited

vlm commented May 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brchiu May 8, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brchiu commented May 8, 2017

brchiu commented May 8, 2017

vlm commented May 8, 2017

vlm commented Jun 27, 2017

brchiu commented May 7, 2017 •

edited

coveralls commented May 7, 2017 •

edited

coveralls commented May 7, 2017 •

edited

coveralls commented May 8, 2017 •

edited

coveralls commented May 8, 2017 •

edited

brchiu May 8, 2017 •

edited