Merge repos?

[mouse07410]

@vlm our codebases are diverging. I've been adding bug fixes (probably faster than you do), and had merged APER support that appears to be working. You added OER support. Also, I merged a significant refactoring PR by @brchiu that simplified the code somewhat.

Neither codebase supports constraints correctly (using INTEGER_t type), but my codebase handles larger constraints because it uses long long. I've tested it with a few simple cases, and users that submitted complaints were satisfied by the fixes.

What are we going to do? Ideally, you could embrace and accept my codebase (assuming you're happy with the results of the tests, of course). Because of the @brchiu PR, it would be quite difficult to cherry-pick and retrofit commits either way.

@brchiu would you be willing to help retrofitting OER support to @mouse07410 codebase? If that is done, @vlm probably could just take back that codebase. Alternatively, somebody would need to re-do APER merging, and all of the constraints fixing...

@vlm, @brchiu, @velichkov - what do you say?

[vlm]

@mouse07410, thank you for all your efforts! I took a bit in already, but certainly the divergence is there and has to be dealt with. Lately I've been busy with IOC and OER because of explicit industry-supported need (connected vehicles project). APER is not one of the short term goals so I did not pay much attention to it lately.

Re-integration effort

A few weeks ago I spent a few days (literally, days) trying to integrate your codebase, and failed along several independent axes. One is that it seems that the "long long" support is incorrect in a few places and just appears to magically work on little-endian platform for simpler examples. This requires redesign rather than reintegration.

INTEGER constraints

With respect to the integer constraints, I intend to redo it by introducing a more granular integer handling (with u/int{8,16,32,64}_t). So this part will be fixed independently.

OER

The OER that I am pursuing is going to evolve very quickly in the next few days (weeks), so there's that.

APER

The APER effort that you (and someone else, independently) created is valuable, but I found hard to integrate it. Because during that integration I have to go through the spec and ensure that the code does what it supposed to do. And it was hard for me to read the code and match it with the spec, not in the least part because the original code is not too clear (haha!). In any case, this has to wait until I finish OER (which has a hard deadline in a few days, so bear with me).

Recap

So overall, my strategy is the following:

1. Your codebase is valuable from my POV in that it plays with APER; I'll be able to tap into it when I get to it (and I already did for a few small things). To what extent is not clear, but having it around is definitely helpful. I'll get to it and integrate it back here.
2. The OER is gonna be evolving and will get done very very soon.
3. The integer constraints I'll fix independently.
4. The rest great bunch of small fixes I would love being reintroduced as several separate, cleaned up and focused PRs. Having them as focused PRs simplifies independent review and quick merging.

[brchiu]

hi, Folks,

Since I am a potential asn1c user for S1AP/RANAP/NBAP, so I'd better do something to make APER function works here.

According to my limited knowledge of APER implementation in #125, most of the changes reside in skeleton directory. After @vlm apply #186, which is a re-implementation of #143 and adds pr 10 of @mouse07410's repository, the difference between @vlm and @mouse07410 repositories' skeleton directories will be smaller.

I can create a new pr and copy those functions with _aper suffix, for example INTEGER_en/decode_aper, OCTET_STRING_en/decode_aper, ..., from mouse07410 repository.

As for newly added OPEN_TYPE.c in @vlm repository, empty APER function stub can be added temporarily for successful compilation.

IMHO, this approach can merge back major part of current APER implementation from @mouse07410 repository back to @vlm's.

Another issue is #185 (S1AP, RANAP and NBAP's failed to be parsed), it has nothing to do with APER but it serve an obstacle for people using these S1AP/RANAP/NBAP protocol. Before @vlm has time for it, I can try to dig into the code and find a solution (or workaround).

I am not acquainted with other changes in @mouse07410 so I won't pay too much attention on merging them here.

[mouse07410]

According to my limited knowledge of APER implementation in #125, most of the changes reside in skeleton directory. After @vlm apply #186, which is a re-implementation of #143 and adds pr 10 of @mouse07410's repository, the difference between @vlm and @mouse07410 repositories' skeleton directories will be smaller.

@vlm and @brchiu I recommend to not use #125. I had it in my master, then had to back off and replace it with a different APER-implementing PR from @AuthenticEshkinKot. It addressed several APER and UPER issues that #125 did not handle correctly, and its submitter did not have time for fixing. It still exists in my repo as "branch 125". ;-)

I hope it is feasible to merge the APER support from my master branch - as it's the most-debugged and the richest capabilities-wise.

I am not acquainted with other changes in @mouse07410 so I won't pay too much attention on merging them here.

Frankly, the biggest change - and the one that brought the greatest incompatibility between these two repos was your https://github.com/mouse07410/asn1c/pull/7. Now in retrospective I wish we haven't done that.

@vlm for merging the small fixes in - would you like to make me a co-maintainer of the "main" repo? For the big ones - I don't dare doing anything on my own.

[brchiu]

@mouse07410 , sorry for my misleading description.

I know there are more changes and fixes for APER function in your repository rather than vanilla #125. I just use it as an illustrative example to show the scope of modification required for APER. If I prepare this aforementioned PR, I definitely will merge code from your master branch.

For mouse07410#7, as I said, it was re-implemented in #186 since @vlm re-structure the tests directories and I have some new thought about it. (They are minor, don't worry, ex. whether to remove function name mapping in header file, delimiter characters usage ... etc) So even #186 is merged, your repository won't be compatible with @vlm's because of large changes done in recent commits.

Honestly speaking, I don't regret merging these PRs earlier because we don't know when @vlm will come back to this project at that time. Of course it's good to see him continue working on it.

[vlm]

@mouse07410, wrt co-maintainer: please sign the appropriate CLA and send the scanned PDF to my email.

[brchiu]

hi @vlm,

By the way, I recently find that the indention of asn1c code become mixing with different style. Perhaps we can use some code formatting tool like astyle or indent to unify them. Moreover, adding a git pre-commit hook to format code before commit ?

Another possible worthy-doing is to use source code scanning tool like Coverity to scan generated code from, for example, J2735 and 1609 standards, which require higher security level. ARM's folks claim their Trusted Firmware was scanned by an on-line (free but limited capability) version of Coverity. Especially if the generated code are to be used in vehicle project which might require MISRA-C check.

Both these are only suggestions for your reference.

[vlm]

The asn1c repository has the .clang-format file which standardizes the code style. I am reluctant to do a sweeping change in the whole codebase, as it will almost instantly provide a tremendous pain and suffering to the folks having their own branches and forks.

So the new code I do is created with that .clang-format style (I have a shortcut in my .vimrc for that), and I encourage other contributors to use it. The tiny changes in the old code formatted with tabs are to be left with the old tabs-laden style. If a large portion of a single function in the older base is getting rewritten, it should move to the new style in its entirety.

This is a bit unfortunate, but I think the style is subordinate to the ease of patch management and working with other contributors.

[mouse07410]

@vlm, for your info. As of a few weeks ago, my repo is again tracking your master - currently with the branch vlm_master, soon to become the master. It merges #226 from @brchiu and some other bug fixes.

[soroshsabz]

ITNOA

@vlm any other update?

[mouse07410]

The default branch of https://GitHub.com/mouse07410/asn1c.git is"vlm_master", which tracks the current master of this repo (including support for OER) and adds enhancements, like APER and multiple bug fixes that weren't merged here (yet?).

[eisaev]

I also encountered errors on 32-bit systems that have already been fixed in mouse07410/asn1c:

per_support.c:238: per_long_range_rebase: Assertion `lb <= ub' failed.

The error is related to the size of the long type on 32-bit systems and was mentioned in several issues. Like many here, I'm also waiting for these fixes to merge into the main repository. Thanks!