summaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* To better match groff parsing, reject digits and some mathematicalIngo Schwarze2022-06-061-7/+9
| | | | | | | | | | | | | | | | | | operators as argument delimiters for some escape sequences that take numerical arguments, in the same way as it had already been done for \h. Argument delimiter parsing for escape sequences taking numerical arguments is not perfect yet. In particular, when a character representing a scaling unit is abused as the argument delimiter, parsing for that character becomes context-dependent, and it is no longer possible to find the end of the escape sequence without calling the full numerical expression parser, which i refrain from attempting in this commit. For now, continuing to misparse insane constructions like \Bc1c+1cc (which is valid in groff and resolves to "1" because 1c+1c = two centimeters is a valid numerical expression and 'c' is also a valid delimiter) is a small price to pay for keeping complexity at bay and for not losing focus in the ongoing series of refinements.
* adjust two desired error messages after roff_escape.c rev. 1.11Ingo Schwarze2022-06-061-2/+2
| | | | improved diagnostics for the \C escape sequence
* Allow arbitrary argument delimiters for \C, like groff does.Ingo Schwarze2022-06-061-4/+5
| | | | | | | | | The restriction of only allowing ' as the delimiter was introduced by kristaps@ on 2011/04/09 when he first supported \C. For most other escape sequences, similar restrictions were relaxed later on, but for the rarely used \C, it was apparently forgotten. While here, reject empty character names: they are never valid.
* add and update a few entriesIngo Schwarze2022-06-061-0/+11
|
* With the improved escape sequence parser, it becomes easy to also improveIngo Schwarze2022-06-0516-96/+147
| | | | | | | | | diagnostics. Distinguish "incomplete escape sequence", "invalid special character", and "unknown special character" from the generic "invalid escape sequence", also promoting them from WARNING to ERROR because incomplete escape sequences are severe syntax violations and because encountering an invalid or unknown special character makes it likely that part of the document content intended by the authors gets lost.
* Small cleanup of error reporting:Ingo Schwarze2022-06-051-11/+12
| | | | | | | call mandoc_msg() only once at the end, not sometimes in the middle, classify incomplete, non-expanding escape sequences as ESCAPE_ERROR, and also reduce the number of return statemants; no formatting change intended.
* During identifier parsing, handle undefined escape sequencesIngo Schwarze2022-06-0318-58/+222
| | | | | | | | | | | | | | | | | | | | | | in the same way as groff: * \\ is always reduced to \ * \. is always reduced to . * other undefined escape sequences are usually reduced to the escape name, for example \G to G, except during the expansion of expanding escape sequences having the standard argument form (in particular \* and \n), in which case the backslash is preserved literally. Yes, this is confusing indeed. For example, the following have the same meaning: * .ds \. and .ds . which is not the same as .ds \\. * \*[\.] and \*[.] which is not the same as \*[\\.] * .ds \G and .ds G which is not the same as .ds \\G * \*[\G] and \*[\\G] which is not the same as \*[G] <- sic! To feel less dirty, have a leaning toothpick, if you are so inclined. This patch also slightly improves the string shown by the "escaped character not allowed in a name" error message.
* Since \. is not a character escape sequence, re-classify it from theIngo Schwarze2022-06-023-11/+15
| | | | | | | | | | | | | wrong parsing class ESCAPE_SPECIAL to the better-suited parsing class ESCAPE_UNDEF, exactly like it is already done for the similar \\, which isn't a character escape sequence either. No formatting change is intended just yet, but this will matter for upcoming improvements in the parser for roff(7) macro, string, and register names. See the node "5.23.2 Copy Mode" in "info groff" regarding what \\ and \. really mean.
* Avoid the layering violation of re-parsing for \E in roff_expand().Ingo Schwarze2022-06-023-31/+25
| | | | | | | | | To that end, add another argument to roff_escape() returning the index of the escape name. This also makes the code in roff_escape() a bit more uniform in so far as it no longer needs the "char esc_name" local variable but now does everything with indices into buf[]. No functional change.
* Fix a buffer overrun in the roff(7) escape sequence parser that couldIngo Schwarze2022-06-011-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | be triggered by macro arguments ending in double backslashes, for example if people wrote .Sq "\\" instead of the correct .Sq "\e". The bug was hard to find because it caused a segfault only very rarely, according to my measurements with a probability of less than one permille. I'm sorry that the first one to hit the bug was an arm64 release build run by deraadt@. Thanks to bluhm@ for providing access to an arm64 machine for debugging purposes. In the end, the bug turned out to be architecture-independent. The reason for the bug was that i assumed an invariant that does not exist. The function roff_parse_comment() is very careful to make sure that the input buffer does not end in an escape character before passing it on, so i assumed this is still true when reaching roff_expand() immediately afterwards. But roff_expand() can also be reached from roff_getarg(), in which case there *can* be a lone escape character at the end of the buffer in case copy mode processing found and converted a double backslash. Fix this by handling a trailing escape character correctly in the function roff_escape(). The lesson here probably is to refrain from assuming an invariant unless verifying that the invariant actually holds is reasonably simple. In some cases, in particular for invariants that are important but not simple, it might also make sense to assert(3) rather than just assume the invariant. An assertion failure is so much better than a buffer overrun...
* Rudimentary implementation of the \A escape sequence, following groffIngo Schwarze2022-05-313-5/+40
| | | | | | | | | | | | | | | | | | | | semantics (test identifier for syntactical validity), not at all following the completely unrelated Heirloom semantics (define hyperlink target position). The main motivation for providing this implementation is to get \A into the parsing class ESCAPE_EXPAND that corresponds to groff parsing behaviour, which is quite similar to the \B escape sequence (test numerical expression for syntactical validity). This is likely to improve parsing of nested escape sequences in the future. Validation isn't perfect yet. In particular, this implementation rejects \A arguments containing some escape sequences that groff allows to slip through. But that is unlikely to cause trouble even in documents using \A for non-trivial purposes. Rejecting the nested escapes in question might even improve robustnest because the rejected names are unlikely to really be usable for practical purposes - no matter that groff dubiously considers them syntactically valid.
* Trivial patch to put the roff(7) \g (interpolate format of register)Ingo Schwarze2022-05-313-2/+5
| | | | | | | | | | | | | escape sequence into the correct parsing class, ESCAPE_EXPAND. Expansion of \g is supposed to work exactly like the expansion of the related escape sequence \n (interpolate register value), but since we ignore the .af (assign output format) request, we just interpolate an empty string to replace the \g sequence. Surprising as it may seem, this actually makes a formatting difference for deviate input like ".O\gNx" which used to raise bogus "escaped character not allowed in a name" and "skipping unknown macro" errors and printed nothing, whereas now it correctly prints "OpenBSD".
* Dummy implementation of the roff(7) \V (interpolate environment variable)Ingo Schwarze2022-05-307-9/+47
| | | | | | | | | escape sequence. This is needed to get \V into the correct parsing class, ESCAPE_EXPAND. It is intentional that mandoc(1) output is *not* influenced by environment variables, so interpolate the name of the variable with some decorating punctuation rather than interpolating its value.
* Re-classify the roff(7) \r (reverse line feed) escape sequenceIngo Schwarze2022-05-206-10/+36
| | | | | | | from "ignore" to "unsupported" because when an input file uses it, mandoc(1) is likely to significantly misformat the output, usually showing parts of the output in a different order than the author intended.
* Test the handling of some additional one-character escape sequencesIngo Schwarze2022-05-203-13/+43
| | | | | that take no argument and are ignored: \% \& \^ \a \d \t \u \{ \| \} No change to parsing or formatting needed.
* following the fixed parsing direction of roff_expand() in roff.c rev. 1.388,Ingo Schwarze2022-05-193-29/+29
| | | | some diagnostics now appear in a more reasonable order, too
* Adjust a column number in an error messageIngo Schwarze2022-05-191-1/+1
| | | | | | | | | | | | | | | | | | | | after the roff_expand() reorganization in roff.c rev. 1.388. The new parsing direction has two effects: 1. Correct output when a line contains more than one expanding escape sequence that has a side effect. 2. Column numbers in diagnostic messages now report the changed column numbers after any expansions left of them have taken place; in the past, column numbers refered to the original input line. Arguably, item 2 was a bit better in its old state, but slightly less helpful diagnostics are a small price to pay for correct output. Besides, when the expansion of user-defined strings or macros is involved, in many cases, mandoc(1) is already unable to report meaningful line and column numbers, so item 2 is not a noteworthy regression. The effort and code complication for fixing that would probably be excessive, in particular since well-written manual pages are not supposed to use such features in the first place.
* fix a wrong column number that got fixed as a side effectIngo Schwarze2022-05-191-1/+1
| | | | of the roff_expand() reorganization in roff.c rev. 1.388
* remove a bogus warning that went away as a side effectIngo Schwarze2022-05-191-1/+0
| | | | of the roff_expand() reorganization in roff.c rev. 1.388
* Make roff_expand() parse left-to-right rather than right-to-left.Ingo Schwarze2022-05-196-646/+677
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Some escape sequences have side effects on global state, implying that the order of evaluation matters. For example, this fixes the long-standing bug that "\n+x\n+x\n+x" after ".nr x 0 1" used to print "321"; now it correctly prints "123". Right-to-left parsing was convenient because it implicitly handled nested escape sequences. With correct left-to-right parsing, nesting now requires an explicit implementation, here solved as follows: 1. Handle nested expanding escape sequences iteratively. When finding one, expand it, then retry parsing the enclosing escape sequence from the beginning, which will ultimately succeed as soon as it no longer contains any nested expanding escape sequences. 2. Handle nested non-expanding escape sequences recursively. When finding one, the escape sequence parser calls itself to find the end of the inner sequence, then continues parsing the outer sequence after that point. This requires the mandoc_escape() function to operate in two different modes. The roff(7) parser uses it in a mode where it generates diagnostics and may return an expansion request instead of a parse result. All other callers, in particular the formatters, use it in a simpler mode that never generates diagnostics and always returns a definite parsing result, but that requires all expanding escape sequences to already have been expanded earlier. The bulk of the code is the same for both modes. Since this required a major rewrite of the function anyway, move it into its own new file roff_escape.c and out of the file mandoc.c, which was misnamed in the first place and lacks a clear focus. As a side benefit, this also fixes a number of assertion failures that tb@ found with afl(1), for example "\n\\\\*0", "\v\-\\*0", and "\w\-\\\\\$0*0". As another side benefit, it also resolves some code duplication between mandoc_escape() and roff_expand() and centralizes all handling of escape sequences (except for expansion) in roff_escape.c, hopefully easing maintenance and feature improvements in the future. While here, also move end-of-input handling out of the complicated function roff_expand() and into the simpler function roff_parse_comment(), making the logic easier to understand. Since this is a major reorganization of a central component of mandoc(1), stability of the program might slightly suffer for a few weeks, but i believe that's not a problem at this point of the release cycle. The new code already satisfies the regression suite, but more tweaking and regression testing to further improve the handling of various escape sequences will likely follow in the near future.
* improve a comment explaining a particularly nasty hack; no code changeIngo Schwarze2022-05-191-1/+6
|
* Split a new function roff_parse_comment() out of roff_expand() because thisIngo Schwarze2022-05-015-99/+154
| | | | | | | functionality is not needed when called from roff_getarg(). This makes the long and complicated function roff_expand() significantly shorter, and also simpler in so far as it no longer needs to return ROFF_APPEND. No functional change intended.
* Provide a new function roff_req_or_macro() to parse and handle a requestIngo Schwarze2022-04-309-43/+160
| | | | | | | | | | | | | | or macro, including context-dependent error handling inside tbl(7) code and inside .ce/.rj blocks. Use it both in the top level roff(7) parser and inside conditional blocks. This fixes an assertion failure triggered by ".if 1 .ce" inside tbl(7) code, found by tb@ using afl(1). As a side benefit for readability, only one place remains in the code that calls the main handler functions for the various roff(7) requests. This patch also improves column numbers in some error messages and various comments.
* Add comments to some of the enum roff_tok values;Ingo Schwarze2022-04-301-12/+12
| | | | | | particularly useful for values that have non-obvious semantics like ROFF_MAX, ROFF_cblock, ROFF_RENAMED, and TOKEN_NONE; no code change.
* Refactor the handler function roff_block_sub() for clarity and simplicity.Ingo Schwarze2022-04-301-16/+9
| | | | | | | | | | | | | 1. Do not needlessly access the function pointer table roffs[]. Instead, simply call the block closing function directly. 2. Sort code: handle both cases of block closing at the beginning of the function rather than one at the beginning and one at the end. 3. Trim excessive, partially repetitive and obvious comments, also making the comments considerably more precise. No functional change.
* The syntax of the roff(7) .mc request is quite specialIngo Schwarze2022-04-289-3/+135
| | | | | | | | | and the roff_onearg() parsing function is too generic, so provide a dedicated parsing function instead. This fixes an assertion failure when an \o escape sequence is passed as the argument; the bug was found by tb@ using afl(1). It also makes mandoc output more similar to groff in various cases.
* Element next-line scopes may nest, so man_breakscope() may have toIngo Schwarze2022-04-285-9/+59
| | | | | | | | break multiple element next-line scopes at the same time, similar to what man_descope() already does for unconditional rewinding. This fixes an assertion failure that tb@ found with afl(1), caused by .SH .I .I .BI and similar sequences of macros without arguments.
* The .AT, .DT, and .UC macros are allowed inside next-line scopeIngo Schwarze2022-04-2712-8/+104
| | | | | | and never produce output at the place of their invocation. Minibugs found while investigating unrelated afl(1) reports from tb@.
* Fix three bugs regarding the interaction of \z and \h:Ingo Schwarze2022-04-276-7/+58
| | | | | | | | | | | | | | | | | | | | 1. The combination \z\h is a no-op whatever the argument may be. In the past, the \z only affected the first space character generated by the \h, which was wrong. 2. For the conbination \zX\h with a positive argument, the first space resulting from the \h is not printed but consumed by the \z. 3. For the combination \zX\h with a negative argument, application of the \z needs to be completed before the \h can be started. In the past, if this combination occurred at the beginning of an output line, the \h backed up to the beginning of the line and after that, the \z attempted to back up even further, triggering an assertion. Bugs found during an audit of assignments to termp->col that i started after the bugfix tbl_term.c rev. 1.65. The assertion triggered by bug 3 was *not* yet found by afl(1).
* typo in example text: unsused -> unused; noticed by tb@Ingo Schwarze2022-04-264-5/+5
|
* At the end of every tbl(7) cell, clear the \z state.Ingo Schwarze2022-04-266-5/+65
| | | | | | | | | This is needed because the TERMP_MULTICOL mode is designed such that term_tbl() buffers all the cells of the table row before the normal reset logic near the end of term_flushln() can be reached. This fixes an assertion failure triggered by \z near the end of a table cell, found by tb@ using afl(1).
* If a node is tagged explicitly, skip implicit tagging for that node.Ingo Schwarze2022-04-267-7/+67
| | | | | | | | Apart from making sense in the first place, this fixes an assertion failure that happened when the calculated implicit tag did not match the string value of the first child of the node, Bug found by tb@ using afl(1).
* When we open a new .while loop, let's not attempt to close outIngo Schwarze2022-04-241-2/+4
| | | | | | | | | another enclosing .while loop at the same time. Instead, postpone the closing until the next iteration of ROFF_RERUN. This prevents one-line constructions like ".while 0 .while 0 something" and ".while rx .while rx .rr x" (which admittedly aren't particularly useful) from dying of abort(3), which was a bug tb@ found with afl(1).
* If a .shift request has a negative argument, do not use a negative arrayIngo Schwarze2022-04-247-11/+32
| | | | | | | | index but use 0 instead of the argument, just like groff. Warn about the invalid argument. While here, fix the column number in another warning message. Segfault reported by tb@, found with afl(1).
* If the last data row of a tbl(7) contains nothing but a horizontal line,Ingo Schwarze2022-04-231-2/+4
| | | | | | | | | | | | | | | | | | | | | | | do not skip closing the table and cleaning up memory at the end of the table in the HTML output module. This bug resulted in skipping the tblcalc() function and reusing the existing roffcol array for the next tbl(7) processed. If the next table had more columns than the one ending with a horizontal line in the last data row, uninitialized memory was read, potentially resulting in near-infinite output. The bug was introduced in rev. 1.29 (2018/11/26) but only fully exposed by rev. 1.38 (2021/09/09). Until rev. 1.37, it could only cause misformatting and invalid HTML output syntax but not huge output because up to that point, the function did not use the roffcol array. Nasty bug found the hard way by Michael Stapelberg on the production server manpages.debian.org. Michael also supplied example files and excellent instructions how to reproduce the bug, which was very difficult because no real-world manual page is known that triggers the bug by itself, so to reproduce the bug, mandoc(1) had to be invoked with at least two file name arguments.
* support for hunting memory leaks;Ingo Schwarze2022-04-1419-66/+848
| | | | designed and written last autumn, polished today
* some HTML/CSS issues from John GardnerIngo Schwarze2022-04-141-1/+12
|
* prefer https links in man pagesIngo Schwarze2022-04-141-3/+3
| | | | | patch from jsg@ ok gnezdo@ miod@ jmc@
* To prevent infinite recursion while expanding eqn(7) definitions,Ingo Schwarze2022-04-135-19/+94
| | | | | | | | | | | | | | | | | | | | | | | | | | | we must not reset the recursion counter when moving beyond the end of the *previous* expansion, but we may only do so when moving beyond the rightmost position reached by *any* expansion in the current equation. This matters because definitions can nest; consider: .EQ define inner "content" define outer "inner outer" outer .EN This endless loop was found by tb@ using afl(1). Incidentally, GNU eqn(1) also performs an infinite loop in this situation and then crashes when memory runs out, but that's not an excuse for nasty behaviour of mandoc(1). While here, consistently print the expanded content even when the expansion is finally truncated. While that is not likely to help end-users, it may help authors of eqn(7) code to understand what's going on. Besides, it sends a very clear signal that something is amiss, which was easy to miss in the past unless people enabled -W error or used -T lint.
* Do not die on an assertion if an input file contains no sectionIngo Schwarze2022-04-134-5/+8
| | | | | | | | | whatsoever and ends with a broken next-line scope. Obviously, this cannot happen in a real manual page, but mandoc(1) should not die even when fed absurd input. This bug was independently reported by both jsg@ and tb@ who both found it with afl(1).
* Surprisingly, groff supports multiple copy mode escapes at theIngo Schwarze2022-04-135-7/+55
| | | | | | | | | | | | | beginning of an escape sequence: \, \E, \EE, \EEE, and so on all do the same outside copy mode, so let them do the same in mandoc(1), too. This fixes an assertion failure triggered by \EE*X that tb@ found with afl(1). The first E was consumed by roff_expand(), but that function failed to recognize the escape sequence as the expansion of a user-defined string and handed it over to mandoc_escape(), which consumed the second E and then died on an assertion because it is not prepared to handle user-defined strings. Fix this by letting *both* functions handle arbitrary numbers of 'E's correctly.
* When calculating the with of spanned columns, which for example mattersIngo Schwarze2022-04-081-2/+5
| | | | | | | | for centering text spanning multiple tbl(7) columns, correctly account for the spacing between columns instead of wrongly assuming the default spacing of 3n. Patch from Simon Branch <simonmbranch at gmail dot com>.
* new TODO entry: handle Unicode letters in tagsIngo Schwarze2022-03-271-0/+5
|
* The demandoc(1) program neither reads nor writes any databases, soIngo Schwarze2022-03-201-2/+1
| | | | | | | | | delete a sentence taking about databases. Having that sentence in the first place probably was a copy-and-paste mistake when adopting some text from the makewhatis(8) manual page. Triggered by a smaller patch sent to discuss@ by Paul A. Patience <paul at apatience dot com>.
* Avoid legacy CSS2 syntax for the "display" property and use the CSS3Ingo Schwarze2022-03-171-3/+3
| | | | | | | | | | two-value syntax "display: inline flow;" instead. In particular, there is no need to establish a new block formatting context with "flow-root", and in fact that's detrimental because it appears to introduce spurious soft-wrap opportunities. jmc@ reported a bogus line break between the opening angle bracket generated by .Aq Mt and the following email address.
* Just say that the databases are intended for use by apropos(1), whatis(1),Ingo Schwarze2022-03-161-4/+2
| | | | | | | | | | | and man(1), without restricting that statement to "man -k". Suggested by and patch OK'ed by jmc@. While only apropos(1) and whatis(1) strictly require the database and while our man(1) implementation can find many manual pages even when no database is available or when the database is incomplete or corrupt, it does use the database even without -k whenever possible. Consequently, this change makes the manual page less confusing.
* In the first example, use "mandoc -a" directly rather "mandoc -l".Ingo Schwarze2022-02-081-1/+1
| | | | | | | | | | | | | It feels more natural to me to use -a directly when asking mandoc(1) to use a pager. The reason that "mandoc -l" does exactly the same as "mandoc -a" is that "mandoc" is essentially "man -lc", so the -a implied by -l negates the -c and the -l has no effect because it is already the default for mandoc(1). The more usual command for doing the same is "man -l foo.1 bar.1 ..." but that's off-topic for the mandoc(1) manual page. Patch on tech@ from Anders Damsgaard <anders at adamsgaard dot dk>.
* remove "please" from manual page;Ingo Schwarze2022-02-081-1/+1
| | | | patch from jsg@, ok jmc@ sthen@ millert@
* Tedu support for the -xsh4.2 argument to the mdoc(7) .St macroIngo Schwarze2022-01-132-7/+3
| | | | | | | | | | | because all of the following hold: * It is an alias for a part of an ancient standard that is no longer important. * To refer to that old standard, -xpg4.2 is readily available and portable. * It is unused in OpenBSD, FreeBSD, and NetBSD. * Groff never supported it. I agreed with G. Branden Robinson that deleting this from mandoc is preferable to adding it to groff.
* Only sort the result array if it contains more than one element,Ingo Schwarze2022-01-131-2/+2
| | | | | | | | | | | | | | | | | | | making the mansearch() function easier to read for human auditors. No functional change on OpenBSD. As observed by Mark Millard <marklmi at yahoo dot com>, neither the latest version of POSIX 2008 nor C11 defines what qsort(3) should do for base == NULL && nmemb == 0. My impression is it is indeed undefined behaviour because the standards say that base shall point to an array, NULL does not point to an array, and while there is special wording saying that compar() shall not be called if nmemb == 0, i fail to see any similar wording stating that base shall not be accessed if nmemb == 0. Consequently, this patch is also likely to improve standard conformance and portability. Minor issue found by Stefan Esser <se at FreeBSD> with UBSAN. He sent a patch to bugs@, but my patch differs in a minor way.