Read a define value untill the end of the line or when it encounters a comment by Miauwkeru · Pull Request #170 · fox-it/dissect.cstruct

Miauwkeru · 2026-07-29T07:56:25Z

Keeps track of the whitespece between the value and the comment
Will keep track of a single qouted character

fixes #169

…a comment - Keeps track of the whitespece between the value and the comment - Will keep track of a single qouted character

codecov · 2026-07-29T07:58:25Z

Codecov Report

❌ Patch coverage is 0% with 35 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.00%. Comparing base (3032ad7) to head (d60d2ac).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
dissect/cstruct/lexer.py	0.00%	35 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff          @@
##            main    #170   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files         22      22           
  Lines       2798    2888   +90     
=====================================
- Misses      2798    2888   +90

Flag	Coverage Δ
unittests	`0.00% <0.00%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Miauwkeru · 2026-07-29T07:59:22Z

This was the first one that got through all the tests, I will think of a better solution for the quoted string as it will break if you add a single quote somewhere between double quotes

codspeed · 2026-07-29T08:00:27Z

Merging this PR will not alter performance

✅ 14 untouched benchmarks

_{Comparing read-define-value-till-comment (d60d2ac) with main (46b850f)}

Miauwkeru · 2026-07-29T11:43:25Z

I can replace it with _read_string() however, this does require to escape:

dissect.cstruct/tests/test_parser.py

Line 254 in 3032ad7

#define QUOTES "\'\"a'b\""

oterwise _read_string can't distinguish the quotes from each other. @Schamper is it an idea to always escape the characters? (and document that this is required) If not, I will look for another way

Schamper · 2026-07-29T11:52:25Z

What about multiline defines with quotes at the end of each (or some) line? Note that I'm not sure how that'd work in C, just a thought. Worth a test.

Miauwkeru · 2026-07-29T12:04:12Z

What about multiline defines with quotes at the end of each (or some) line? Note that I'm not sure how that'd work in C, just a thought. Worth a test.

Do you mean something like this:

#define HELP " \
#define NLD "

it won't work without the backslash according to some tests with godbolt

Schamper · 2026-07-30T10:03:07Z

I mean something like this:

#define foo bar \  // comment here
            zomg \  // and here
            yolo

Miauwkeru · 2026-07-30T10:54:53Z

I mean something like this:

#define foo bar \  // comment here
            zomg \  // and here
            yolo

it will error in that case in c:

main.c:4:13: error: expected identifier or ‘(’ before string constant
    4 |             "zomg" \  // and here
      |             ^~~~~~
main.c:4:20: error: stray ‘\’ in program
    4 |             "zomg" \  // and here
      |                    ^
main.c:3:19: error: stray ‘\’ in program
    3 | #define foo "bar" \  // comment here
      |                   ^
main.c:8:12: note: in expansion of macro ‘foo’
    8 |     printf(foo);

and in cstruct it creates a lexer error for an unexpected \\ character next to zomg

Schamper · 2026-08-01T09:10:29Z

This is valid C though:

#define foo "bar" /* comment */ \
        "zomg"

Might be nice to at least put something like this in the unit test.

Miauwkeru · 2026-08-03T08:38:44Z

This is valid C though:
#define foo "bar" /* comment */ \
        "zomg"
Might be nice to at least put something like this in the unit test.

I'll add it and see whether I can make that work :)

Miauwkeru · 2026-08-10T08:44:44Z

In C there is also the case that you can concatenate two defines if they have the same type. This doesn't work yet as it doesn't resolve yet. I think it might be better to put that functionality in a different pr if we want it.

def test_preprocessor_define_substitute_define(cs: cstruct) -> None:
    """Test whether we can concatinate defines with other strings."""
    c_def = """
    #define DATA1 "world"
    #define CONCAT "hello" \\
            DATA1
    """
    cs.load(c_def)

    assert cs.consts["CONCAT"] == "helloworld"

Was because it recognized 'b"' as a word, and attempted to parse the remaining part as normal

william-billaud · 2026-10-09T19:17:17Z

+    assert cs.consts["IGNORE_DEFINE_IN_COMMENT"] == "data4"
+    assert cs.consts["TEXT_CONTAINING_COMMENT"] == "text with comments /* data5 */"
+    assert cs.consts["IGNORE_STRING_AFTER_COMMENT"] == "data6"
+    assert cs.consts["NO_COMMENT_BETWEEN_CONCATINATION"] == 'data7" "should get concatinated'


From my understanting and test the value should be ``'data7should get concatinated'and not'data7" "should get concatinated'`

E.g link on compiler exporer/godbolt

❯ cat test.c #include <stdio.h> #define NO_COMMENT_BETWEEN_CONCATINATION "data7" /* Comment */ \ "should get concatinated" int main(void) { printf(NO_COMMENT_BETWEEN_CONCATINATION); printf("\n"); return 0; } ❯ gcc test.c ❯ ./a.out data7should get concatinated

william-billaud · 2026-10-09T19:23:30Z

+    assert cs.consts["TEXT_CONTAINING_COMMENT"] == "text with comments /* data5 */"
+    assert cs.consts["IGNORE_STRING_AFTER_COMMENT"] == "data6"
+    assert cs.consts["NO_COMMENT_BETWEEN_CONCATINATION"] == 'data7" "should get concatinated'
+


Suggested change

assert "TEST_NO" not in cs.consts

Ensure TEST_NO is actually ignored

william-billaud · 2026-10-09T19:26:01Z

+    assert cs.consts["IGNORE_DEFINE_IN_COMMENT"] == "data4"
+    assert cs.consts["TEXT_CONTAINING_COMMENT"] == "text with comments /* data5 */"
+    assert cs.consts["IGNORE_STRING_AFTER_COMMENT"] == "data6"
+    assert cs.consts["NO_COMMENT_BETWEEN_CONCATINATION"] == 'data7" "should get concatinated'


Suggested change

assert cs.consts["NO_COMMENT_BETWEEN_CONCATINATION"] == 'data7" "should get concatinated'

assert cs.consts["NO_COMMENT_BETWEEN_CONCATINATION"] == '"data7should get concatinated"'

This is true, I currently kept them separate in the define parsing because of a different issue:

#define DATA1 "world" #define CONCAT "hello" \ DATA1

where CONCAT should ideally resolve to helloworld. It currently doesn't of course and resolves to hello DATA1 where it cannot resolve DATA1 yet.
If we remove / join the strings, it would result in helloDATA1 and we would not be a clear separation of the string part and the part that needs to be resolved. So with that in mind, i decided to keep it separate for now.

william-billaud

Few comment/question.

I will try to propose a fix for raised issues next week, as some suggestion make test fail, does not hesitates to tell me if you do not consider these points as valid issues.

Sorry for messy comment order, I'm not used to github suggestion interface.

william-billaud · 2026-10-09T19:34:06Z

+    #define IGNORE_DEFINE_IN_COMMENT    data4      /* define
+    #define TEST_NO                     data_no       inside
+                                                      comment
+                                                    */


Technically this does not compile as it is compiler explorer link

Maybe change with integer for something more correct in C?

only using L530 does indeed not compile, as the comment is not closed. using the full comment does work no? https://godbolt.org/z/zvfT8T4cT

The issue was more related to the quoted /unquoted 'data4' vs '"data4"' (I was not really clear), but this is related to #170 (comment)

oh that, yeah cstruct sees everything as a string after the define. I think we kept it that way for backwards compatibility with <5.0

william-billaud · 2026-10-09T20:03:55Z

+    assert cs.consts["IGNORE_MULTILINE_COMMENT"] == "data3"
+    assert cs.consts["IGNORE_DEFINE_IN_COMMENT"] == "data4"
+    assert cs.consts["TEXT_CONTAINING_COMMENT"] == "text with comments /* data5 */"
+    assert cs.consts["IGNORE_STRING_AFTER_COMMENT"] == "data6"


We lost the difference between quoted and unquoted value (which is the difference between and int, a char and a string. I don't known dissect.cstruct enough but a first glance it's look like a potential issue

Suggested change

assert cs.consts["IGNORE_STRING_AFTER_COMMENT"] == "data6"

assert cs.consts["IGNORE_STRING_AFTER_COMMENT"] == '"data6"'

# to add in cdef

#define A_SIMPLE_INT 1 //

#define A_INT_AS_STRING "1"

#define A_INT_AS_CHAR '1'

assert cs.consts["A_SIMPLE_INT"] == "1"

assert cs.consts["A_INT_AS_STRING"] == '"1"'

assert cs.consts["A_INT_AS_CHAR"] == "'1'"

i will take a look whether that will be an issue

There is no difference between char and string in cstruct. As for an int, it is properly resolved:

Python 3.14.3 (main, Feb 3 2026, 22:52:18) [Clang 21.1.4 ] on linux Type "help", "copyright", "credits" or "license" for more information. >>> from dissect.cstruct import cstruct >>> cstruct() <dissect.cstruct.cstruct.cstruct object at 0x7c966b671a90> >>> x = cstruct() >>> xxx = """ ... #define A_SIMPLE_INT 1 // ... #define A_INT_AS_STRING "1" ... #define A_INT_AS_CHAR '1' ... """ >>> x.load(xxx) <dissect.cstruct.cstruct.cstruct object at 0x7c966b732710> >>> x.A_SIMPLE_INT 1 >>> x.A_INT_AS_STRING '1' >>> x.A_INT_AS_CHAR '1'

william-billaud · 2026-10-09T20:06:06Z

+    assert cs.consts["IGNORE_LINE_COMMENT"] == "data2"
+    assert cs.consts["IGNORE_MULTILINE_COMMENT"] == "data3"
+    assert cs.consts["IGNORE_DEFINE_IN_COMMENT"] == "data4"
+    assert cs.consts["TEXT_CONTAINING_COMMENT"] == "text with comments /* data5 */"


Cf comment below, I didn't find how to make a suggestion edit from Github gui for two lines....

Suggested change

assert cs.consts["TEXT_CONTAINING_COMMENT"] == "text with comments /* data5 */"

assert cs.consts["TEXT_CONTAINING_COMMENT"] == '"text with comments /* data5 */"'

Read a define value untill the end of the line or when it encounters …

eb8e620

…a comment - Keeps track of the whitespece between the value and the comment - Will keep track of a single qouted character

Miauwkeru marked this pull request as draft July 29, 2026 08:01

Miauwkeru mentioned this pull request Jul 29, 2026

Compatibility with the new dissect.cstruct fox-it/dissect.target#1886

Open

Add concatination of string values

e633073

Add fix for space in a bytestring

d60d2ac

Was because it recognized 'b"' as a word, and attempted to parse the remaining part as normal

Miauwkeru marked this pull request as ready for review October 7, 2026 07:47

william-billaud reviewed Oct 9, 2026

View reviewed changes

	assert cs.consts["NO_COMMENT_BETWEEN_CONCATINATION"] == 'data7" "should get concatinated'
	assert cs.consts["NO_COMMENT_BETWEEN_CONCATINATION"] == '"data7should get concatinated"'

-    assert cs.consts["IGNORE_STRING_AFTER_COMMENT"] == "data6"
+    assert cs.consts["IGNORE_STRING_AFTER_COMMENT"] == '"data6"'
+   # to add in cdef
+    #define A_SIMPLE_INT         1      //
+    #define A_INT_AS_STRING        "1"
+    #define A_INT_AS_CHAR        '1'
+    assert cs.consts["A_SIMPLE_INT"] == "1"
+    assert cs.consts["A_INT_AS_STRING"] == '"1"'
+    assert cs.consts["A_INT_AS_CHAR"] == "'1'"

	assert cs.consts["TEXT_CONTAINING_COMMENT"] == "text with comments /* data5 */"
	assert cs.consts["TEXT_CONTAINING_COMMENT"] == '"text with comments /* data5 */"'

Conversation

Miauwkeru commented Jul 29, 2026

Uh oh!

codecov Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Miauwkeru commented Jul 29, 2026

Uh oh!

codspeed Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

Miauwkeru commented Jul 29, 2026

Uh oh!

Schamper commented Jul 29, 2026

Uh oh!

Miauwkeru commented Jul 29, 2026

Uh oh!

Schamper commented Jul 30, 2026

Uh oh!

Miauwkeru commented Jul 30, 2026

Uh oh!

Schamper commented Aug 1, 2026

Uh oh!

Miauwkeru commented Aug 3, 2026

Uh oh!

Miauwkeru commented Aug 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

william-billaud Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

william-billaud Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

william-billaud left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

william-billaud Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

william-billaud Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Jul 29, 2026 •

edited

Loading

codspeed Bot commented Jul 29, 2026 •

edited

Loading

william-billaud Oct 9, 2026 •

edited

Loading

william-billaud Oct 9, 2026 •

edited

Loading

william-billaud left a comment •

edited

Loading

william-billaud Oct 9, 2026 •

edited

Loading

william-billaud Oct 9, 2026 •

edited

Loading