Skip to content

IRSA-7633 prevent strings from being misclassified as arrays in variant visitor#5

Closed
sreenivasapydi wants to merge 2 commits intoCaltech-IPAC:masterfrom
sreenivasapydi:IRSA-6922
Closed

IRSA-7633 prevent strings from being misclassified as arrays in variant visitor#5
sreenivasapydi wants to merge 2 commits intoCaltech-IPAC:masterfrom
sreenivasapydi:IRSA-6922

Conversation

@sreenivasapydi
Copy link
Copy Markdown

No description provided.

@sreenivasapydi sreenivasapydi changed the title prevent strings from being misclassified as arrays in variant visitor IRSA-7633 prevent strings from being misclassified as arrays in variant visitor Mar 17, 2026
@judith-ipac
Copy link
Copy Markdown
Collaborator

judith-ipac commented Mar 18, 2026

Hi, @sreenivasapydi. Looks good.

Please check whether making this change to json5_parser/json5_parser_reader_template.h addresses the compiler warnings:

@@ -140,7 +140,7 @@ String_type substitute_esc_chars(typename String_type::const_iterator begin,
                                  typename String_type::const_iterator end) {
     typedef typename String_type::const_iterator Iter_type;
 
-    if (end - begin < 2) return String_type(begin, end);
+    if (end < begin + 2) return String_type(begin, end);
 

If it looks good to you, would you mind adding this change to your branch? Thanks.

@judith-ipac judith-ipac self-requested a review March 18, 2026 16:40
Copy link
Copy Markdown
Collaborator

@judith-ipac judith-ipac left a comment

Choose a reason for hiding this comment

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

Looks good. Please see my suggestion about addressing compiler warnings.

@judith-ipac
Copy link
Copy Markdown
Collaborator

p.s. It would be nice if your commit comment mentioned "Port to Debian 12".

@judith-ipac
Copy link
Copy Markdown
Collaborator

Just saw your second commit. Is there a reason why you chose to make that rather complicated change to json5_parser/json5_parser_reader_template.h rather than the simpler one I suggested?

Guard String_type(begin, end) construction with an explicit
begin >= end check so GCC can prove the range is non-negative.
typename String_type::const_iterator end) {
typedef typename String_type::const_iterator Iter_type;

if (begin >= end) return String_type();
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The replaces the earlier complex change. This is a simpler change. Generated and summarized by AI

Guard String_type(begin, end) construction with an explicit
begin >= end check so GCC can prove the range is non-negative.

The earlier changes were ai-generated too, not sure why it was complex though.

Copy link
Copy Markdown
Author

@sreenivasapydi sreenivasapydi left a comment

Choose a reason for hiding this comment

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

used a simple code change

@sreenivasapydi
Copy link
Copy Markdown
Author

I revised the commit and force pushed. please review now.

Hi, @sreenivasapydi. Looks good.

Please check whether making this change to json5_parser/json5_parser_reader_template.h addresses the compiler warnings:

@@ -140,7 +140,7 @@ String_type substitute_esc_chars(typename String_type::const_iterator begin,
                                  typename String_type::const_iterator end) {
     typedef typename String_type::const_iterator Iter_type;
 
-    if (end - begin < 2) return String_type(begin, end);
+    if (end < begin + 2) return String_type(begin, end);
 

If it looks good to you, would you mind adding this change to your branch? Thanks.

@sreenivasapydi
Copy link
Copy Markdown
Author

sorry for the multiple comments, I couldn't find this line :

if (end - begin < 2) return String_type(begin, end);

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.

2 participants