Skip to content

Commit

Permalink
Simplify and fix handling of newline in code span.
Browse files Browse the repository at this point in the history 
Fixes #223 properly (one corner case has been unnoticed/hidden due test
suite normalization feature).

Fixes #230 (strictly speaking duplicate of the corner case).
  • Loading branch information
@mity
mity committed Jan 25, 2024
1 parent d082cdd commit aeddaf5
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 18 deletions.
20 changes: 3 additions & 17 deletions src/md4c.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2506,7 +2506,6 @@ struct MD_MARK_tag {
#define MD_MARK_RESOLVED 0x10 /* Resolved in any definite way. */

/* Mark flags specific for various mark types (so they can share bits). */
#define MD_MARK_CODE_EXTRA_SPACE 0x20
#define MD_MARK_EMPH_OC 0x20 /* Opener/closer mixed candidate. Helper for the "rule of 3". */
#define MD_MARK_EMPH_MOD3_0 0x40
#define MD_MARK_EMPH_MOD3_1 0x80
Expand Down Expand Up @@ -2754,8 +2753,6 @@ md_is_code_span(MD_CTX* ctx, const MD_LINE* lines, int n_lines, OFF beg,
int has_space_before_closer = FALSE;
int has_eol_before_closer = FALSE;
int has_only_space = TRUE;
int opener_needs_extra_space = FALSE;
int closer_needs_extra_space = FALSE;
int line_index = 0;

line_end = lines[0].end;
Expand Down Expand Up @@ -2842,27 +2839,20 @@ md_is_code_span(MD_CTX* ctx, const MD_LINE* lines, int n_lines, OFF beg,
if(has_space_before_closer)
closer_beg--;
else {
/* Go back to the end of prev line */
closer_beg = lines[line_index-1].end;
/* We need to eat the preceding "\r\n" but not any line trailing
* spaces. */
/* But restore any trailing whitespace */
while(closer_beg < ctx->size && ISBLANK(closer_beg))
closer_beg++;
}
} else {
if(has_eol_after_opener)
opener_needs_extra_space = TRUE;
if(has_eol_before_closer)
closer_needs_extra_space = TRUE;
}

opener->ch = _T('`');
opener->beg = opener_beg;
opener->end = opener_end;
opener->flags = (opener_needs_extra_space ? MD_MARK_CODE_EXTRA_SPACE : 0);
closer->ch = _T('`');
closer->beg = closer_beg;
closer->end = closer_end;
closer->flags = (closer_needs_extra_space ? MD_MARK_CODE_EXTRA_SPACE : 0);
return TRUE;
}

Expand Down Expand Up @@ -4248,11 +4238,7 @@ md_process_inlines(MD_CTX* ctx, const MD_LINE* lines, int n_lines)
if(mark->flags & MD_MARK_OPENER) {
MD_ENTER_SPAN(MD_SPAN_CODE, NULL);
text_type = MD_TEXT_CODE;
if(mark->flags & MD_MARK_CODE_EXTRA_SPACE)
MD_TEXT(text_type, _T(" "), 1);
} else {
if(mark->flags & MD_MARK_CODE_EXTRA_SPACE)
MD_TEXT(text_type, _T(" "), 1);
MD_LEAVE_SPAN(MD_SPAN_CODE, NULL);
text_type = MD_TEXT_NORMAL;
}
Expand Down Expand Up @@ -4454,7 +4440,7 @@ md_process_inlines(MD_CTX* ctx, const MD_LINE* lines, int n_lines)
MD_TEXT(text_type, STR(tmp), off-tmp);

/* and new lines are transformed into single spaces. */
if(prev_mark->end < off && off < mark->beg)
if(off == line->end)
MD_TEXT(text_type, _T(" "), 1);
} else if(text_type == MD_TEXT_HTML) {
/* Inside raw HTML, we output the new line verbatim, including
Expand Down
2 changes: 1 addition & 1 deletion test/spec-latex-math.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ $$
f(x) dx
$$
.
<p><x-equation type="display">\int_a^b f(x) dx </x-equation></p>
<p><x-equation type="display"> \int_a^b f(x) dx </x-equation></p>
.
--flatex-math
````````````````````````````````
Expand Down

5 comments on commit aeddaf5

@step-
Copy link
Contributor

@step- step- commented on aeddaf5 Jan 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

There must be some funny pointer somewhere because I'm seeing weird and inconsistent results, for example:

# md2html /tmp/md.md
<p><code>a b</code></p>
<p><code> c d<code></p>
<p><code>e f <code></p>
<p><code>x y z<code></p>
# md2html /tmp/md.md
<p><code>a b</code></p>
<p><code> c d</code></p>
<p><code>e f </code></p>
<p><code>x y z</code></p>

See how the closing code tags are missing sometimes? Can you reproduce?

Test subject:

`
   a
   b
`

`
c
d`

`e
f
`

`
 x
y
 z
`

@mity
Copy link
Owner Author

@mity mity commented on aeddaf5 Jan 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Ayyy, good catch. Cannot reproduce, but there's an uninitialized var now. Sadly, gcc warns only in release build about that.

Should be fixed in 5178c58.

@step-
Copy link
Contributor

@step- step- commented on aeddaf5 Jan 26, 2024

Choose a reason for hiding this comment

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

I've een 5178c58 but I don't have time to test it now. However, here's valgrind's report for aeddaf5. It actually reports a different line as the location of the uninitialized value.


==5250== Conditional jump or move depends on uninitialised value(s)
==5250==    at 0x41B6CC: md_process_inlines (md4c.c:4238)
==5250==    by 0x41D2C2: md_process_normal_block_contents (md4c.c:4682)
==5250==    by 0x41DB3D: md_process_leaf_block (md4c.c:4860)
==5250==    by 0x41DF08: md_process_all_blocks (md4c.c:4942)
==5250==    by 0x4221D7: md_process_doc (md4c.c:6396)
==5250==    by 0x4223DA: md_parse (md4c.c:6465)
==5250==    by 0x424E13: md_html (p2-render.c:1046)
==5250==    by 0x4257E5: p2_parse (p2-parse.c:1865)
==5250==    by 0x40DEFA: egg_markdown_parse (p2-parse.c:53)
==5250==    by 0x429443: stdout_output (help-viewer.c:799)
==5250==    by 0x429B3C: main (help-viewer.c:1035)
==5250==  Uninitialised value was created by a stack allocation
==5250==    at 0x416D31: md_collect_marks (md4c.c:2966)
==5250==
==5250== Conditional jump or move depends on uninitialised value(s)
==5250==    at 0x41C51A: md_process_inlines (md4c.c:4431)
==5250==    by 0x41D2C2: md_process_normal_block_contents (md4c.c:4682)
==5250==    by 0x41DB3D: md_process_leaf_block (md4c.c:4860)
==5250==    by 0x41DF08: md_process_all_blocks (md4c.c:4942)
==5250==    by 0x4221D7: md_process_doc (md4c.c:6396)
==5250==    by 0x4223DA: md_parse (md4c.c:6465)
==5250==    by 0x424E13: md_html (p2-render.c:1046)
==5250==    by 0x4257E5: p2_parse (p2-parse.c:1865)
==5250==    by 0x40DEFA: egg_markdown_parse (p2-parse.c:53)
==5250==    by 0x429443: stdout_output (help-viewer.c:799)
==5250==    by 0x429B3C: main (help-viewer.c:1035)
==5250==  Uninitialised value was created by a stack allocation
==5250==    at 0x416D31: md_collect_marks (md4c.c:2966)
==5250==
==5250== HEAP SUMMARY:
==5250==     in use at exit: 299,058 bytes in 5,070 blocks
==5250==   total heap usage: 9,013 allocs, 3,943 frees, 603,728 bytes allocated
==5250==
==5250== LEAK SUMMARY:
==5250==    definitely lost: 0 bytes in 0 blocks
==5250==    indirectly lost: 0 bytes in 0 blocks
==5250==      possibly lost: 0 bytes in 0 blocks
==5250==    still reachable: 296,298 bytes in 5,047 blocks
==5250==         suppressed: 0 bytes in 0 blocks
==5250== ERROR SUMMARY: 7 errors from 2 contexts (suppressed: 0 from 0)

@mity
Copy link
Owner Author

@mity mity commented on aeddaf5 Jan 26, 2024

Choose a reason for hiding this comment

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

That should be it. Valgrind does not report where that uninitialized variable should be initialized on the 1st place, it reports where it is to be used.

And yes the fix was about making sure all bits of MD_MARK::flags are initialized for the code span marks which is exactly what's evaluated on those lines reported by Valgrind.

@step-
Copy link
Contributor

@step- step- commented on aeddaf5 Jan 26, 2024

Choose a reason for hiding this comment

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

That was it. Later commit 5178c58 passed my test suite. Thanks.

Please sign in to comment.