Skip to content

Commit

Permalink
Update OCTET_STRING_aper.c
Browse files Browse the repository at this point in the history 
Uncomment processing of APC_CONSTRAINTS (line 75)
  • Loading branch information
@mouse07410
mouse07410 authored May 22, 2024
1 parent 940dd5f commit 5fa129c
Showing 1 changed file with 0 additions and 2 deletions.
2 changes: 0 additions & 2 deletions skeletons/OCTET_STRING_aper.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,8 @@ OCTET_STRING_decode_aper(const asn_codec_ctx_t *opt_codec_ctx,
case ASN_OSUBV_ANY:
case ASN_OSUBV_STR:
canonical_unit_bits = unit_bits = 8;
/*
if(cval->flags & APC_CONSTRAINED)
unit_bits = cval->range_bits;
*/
bpc = OS__BPC_CHAR;
break;
case ASN_OSUBV_U16:
Expand Down

8 comments on commit 5fa129c

@CedricRouxEurecom
Copy link

Choose a reason for hiding this comment

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

Hi,

I am from the openairinteface team and this change breaks our CU/DU split.

I can provide more information later, but what motivates this change ?

Thanks.

Regards,
Cédric.

@mouse07410
Copy link
Owner Author

@mouse07410 mouse07410 commented on 5fa129c May 26, 2024 via email

Choose a reason for hiding this comment

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

@vladislav1q
Copy link

@vladislav1q vladislav1q commented on 5fa129c Jul 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This change broken decoding at oai of S1AP setup response from SRS EPC.

@mouse07410
Copy link
Owner Author

@mouse07410 mouse07410 commented on 5fa129c Aug 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This change broken decoding at oai of S1AP setup response from SRS EPC.

Is this the faulty encoder's fault, or a decoder problem?

Reason I'm asking: if constraints were defined - should they not be respected and taken into account? The code before this change simply ignored constraints, aka (IMH) accommodated encoders that did not do a good job.

What is supposed to be the semantic meaning of cval->range_bits?

Edit
Looking at the code, the asn1c APER encoder always sets cval->range_bits to 8 for ASN_OSUBV_STR, regardless. I wonder what other values other encoders set cval->range_bits to. In any case, for ASN_OSUBV_STR it seems that 8 is the only legal and reasonable value.

@KIN92, please reply with info: what encoder produced the APER that required cval->range_bits setting other than 8. Without that info I'm going to revert this commit.

@vladislav1q I'm about to push a change to skeletons/OCTET_STRING_aper.c that fixes some problems in the switch() {} in both encoder and decoder. I'll let you know when I finish that - and will appreciate if you could test the new code then.

@mouse07410
Copy link
Owner Author

Choose a reason for hiding this comment

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

OK, I've reverted this commit - it appears to be wrong, as for APER each encoded octet is supposed to be padded to to 8 bits, not the entire string.

@shuimoshusheng
Copy link

Choose a reason for hiding this comment

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

Are these 8 bits mentioned in the standard document? I just tested vlm_master and it can encode and decode XML normally. After encoding PrintableString, it is one byte less than the old version.

@mouse07410
Copy link
Owner Author

@mouse07410 mouse07410 commented on 5fa129c Aug 20, 2024 via email

Choose a reason for hiding this comment

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

@shuimoshusheng
Copy link

Choose a reason for hiding this comment

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

Sorry, the way/place it added this comment - I can't either see it or respond properly on Github. In short, I don't have a PER standard definition. Wiki says every character should be padded to 8 bits for APER. Wiki can be wrong, of course. On Aug 19, 2024, at 22:31, shuimo @.> wrote:  Are these 8 bits mentioned in the standard document? I just tested vlm_master and it can encode and decode XML normally. After encoding PrintableString, it is one byte less than the old version. — Reply to this email directly, view it on GitHub<5fa129c#commitcomment-145539957>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABNGF6MX5OIH4MZCE734PBDZSKTA5AVCNFSM6AAAAABIJFOY2GVHI2DSMVQWIX3LMV43OQ3PNVWWS5CDN5WW2ZLOOQ5TCNBVGUZTSOJVG4. You are receiving this because you commented.Message ID: @.>

In this file ITU-Z.14 14.15.1chapter, Octet is set 8 Bitstring.
image

So please confirm if it is always set to 8 bits in 5fa129c。

Please sign in to comment.