gbranden pushed a commit to branch master
in repository groff.
commit 5dba2b9605ac8c86081a8244508ec97ed3193f08
Author: G. Branden Robinson <[email protected]>
AuthorDate: Sun Apr 20 17:22:30 2025 -0500
[troff]: Slightly refactor (`FONT_NOT_MOUNTED`).
Use a manifest constant for failures to look up a font. Aping
libc-style, `errno`-using functions conceals more than it reveals here
since we have no analog to `errno`. (`errno` was a blind alley anyway,
being as reentrant and concurrency-friendly as a global lock, but worse
because it led to indeterminate program states instead of
slow-but-correct operation. Option types, error-storing function
parameters, and exceptions are all better solutions.)
* src/roff/troff/node.h: Declare `FONT_NOT_MOUNTED`.
* src/roff/troff/node.cpp: Define it.
* src/roff/troff/node.cpp (font_lookup_info::font_lookup_info)
(troff_output_file::really_begin_page)
(troff_output_file::really_copy_file)
(font_family::font_family)
(font_family::resolve)
(font_family::invalidate_fontno)
(has_font)
(symbol_fontno): Use it instead of a `-1` literal.
* src/roff/troff/env.cpp (environment::set_font)
(environment::set_family)
(environment::environment): Determine font lookup failure by explicit
comparison to `FONT_NOT_MOUNTED` instead of checking for any negative
value.
(font_family::resolve): Add assertion to spit up if caller tries to
smuggle us a nonsense mounting position.
---
ChangeLog | 30 ++++++++++++++++++++++++++++++
src/roff/troff/env.cpp | 10 +++++-----
src/roff/troff/node.cpp | 41 ++++++++++++++++++-----------------------
src/roff/troff/node.h | 1 +
4 files changed, 54 insertions(+), 28 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 227999a9b..dcf5c665b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,33 @@
+2025-04-20 G. Branden Robinson <[email protected]>
+
+ [troff]: Slightly refactor. Use a manifest constant for
+ failures to look up a font. Aping libc-style, `errno`-using
+ functions conceals more than it reveals here since we have no
+ analog to `errno`. (`errno` was a blind alley anyway, being as
+ reentrant and concurrency-friendly as a global lock, but worse
+ because it led to indeterminate program states instead of
+ slow-but-correct operation. Option types, error-storing
+ function parameters, and exceptions are all better solutions.)
+
+ * src/roff/troff/node.h: Declare `FONT_NOT_MOUNTED`.
+ * src/roff/troff/node.cpp: Define it.
+ * src/roff/troff/node.cpp (font_lookup_info::font_lookup_info)
+ (troff_output_file::really_begin_page)
+ (troff_output_file::really_copy_file)
+ (font_family::font_family)
+ (font_family::resolve)
+ (font_family::invalidate_fontno)
+ (has_font)
+ (symbol_fontno): Use it instead of a `-1` literal.
+
+ * src/roff/troff/env.cpp (environment::set_font)
+ (environment::set_family)
+ (environment::environment): Determine font lookup failure by
+ explicit comparison to `FONT_NOT_MOUNTED` instead of checking
+ for any negative value.
+ (font_family::resolve): Add assertion to spit up if caller tries
+ to smuggle us a nonsense mounting position.
+
2025-04-20 G. Branden Robinson <[email protected]>
[troff]: Trivially refactor.
diff --git a/src/roff/troff/env.cpp b/src/roff/troff/env.cpp
index 4786579a0..0f12947c5 100644
--- a/src/roff/troff/env.cpp
+++ b/src/roff/troff/env.cpp
@@ -586,7 +586,7 @@ bool environment::set_font(symbol nm)
if (is_device_ps_or_pdf)
warn_if_font_name_deprecated(nm);
if (nm == symbol("P") || nm.is_empty()) {
- if (family->resolve(prev_fontno) < 0)
+ if (family->resolve(prev_fontno) == FONT_NOT_MOUNTED)
return false;
int tem = fontno;
fontno = prev_fontno;
@@ -600,7 +600,7 @@ bool environment::set_font(symbol nm)
if (!mount_font(n, nm))
return false;
}
- if (family->resolve(n) < 0)
+ if (family->resolve(n) == FONT_NOT_MOUNTED)
return false;
fontno = n;
}
@@ -635,7 +635,7 @@ void environment::set_family(symbol fam)
if (fam.is_null() || fam.is_empty()) {
int previous_mounting_position = prev_family->resolve(fontno);
assert(previous_mounting_position >= 0);
- if (previous_mounting_position < 0)
+ if (previous_mounting_position == FONT_NOT_MOUNTED)
return;
font_family *tem = family;
family = prev_family;
@@ -650,7 +650,7 @@ void environment::set_family(symbol fam)
(0 != "font family dictionary lookup"));
if (0 /* nullptr */ == f)
return;
- if (f->resolve(fontno) < 0) {
+ if (f->resolve(fontno) == FONT_NOT_MOUNTED) {
error("no font family named '%1' exists", fam.contents());
return;
}
@@ -816,7 +816,7 @@ environment::environment(symbol nm)
prev_fontno = fontno = 1;
if (!is_good_fontno(1))
fatal("font mounted at position 1 is not valid");
- if (family->resolve(1) < 0)
+ if (family->resolve(1) == FONT_NOT_MOUNTED)
fatal("invalid default font family '%1'",
default_family.contents());
prev_fontno = fontno;
diff --git a/src/roff/troff/node.cpp b/src/roff/troff/node.cpp
index b23cba5f3..19762a540 100644
--- a/src/roff/troff/node.cpp
+++ b/src/roff/troff/node.cpp
@@ -98,6 +98,8 @@ struct special_font_list {
special_font_list *global_special_fonts;
static int global_ligature_mode = 1; // three-valued Boolean :-|
static bool global_kern_mode = true;
+// Font mounting positions are non-negative integers.
+const int FONT_NOT_MOUNTED = -1;
class track_kerning_function {
int non_zero;
@@ -120,8 +122,8 @@ struct font_lookup_info {
font_lookup_info();
};
-font_lookup_info::font_lookup_info() : position(-1),
- requested_position(-1), requested_name(0)
+font_lookup_info::font_lookup_info() : position(FONT_NOT_MOUNTED),
+ requested_position(FONT_NOT_MOUNTED), requested_name(0)
{
}
@@ -1562,7 +1564,7 @@ void troff_output_file::really_begin_page(int pageno,
vunits page_length)
else
has_page_begun = true;
current_tfont = 0;
- current_font_number = -1;
+ current_font_number = FONT_NOT_MOUNTED;
current_size = 0;
// current_height = 0;
// current_slant = 0;
@@ -1597,7 +1599,7 @@ void troff_output_file::really_copy_file(hunits x, vunits
y,
must_update_drawing_position = true;
current_size = 0;
current_tfont = 0;
- current_font_number = -1;
+ current_font_number = FONT_NOT_MOUNTED;
for (int i = 0; i < nfont_positions; i++)
font_position[i] = NULL_SYMBOL;
}
@@ -6598,7 +6600,7 @@ font_family::font_family(symbol s)
{
map = new int[map_size];
for (int i = 0; i < map_size; i++)
- map[i] = -1;
+ map[i] = FONT_NOT_MOUNTED;
}
font_family::~font_family()
@@ -6609,25 +6611,19 @@ font_family::~font_family()
// Resolve a requested font mounting position to a mounting position
// usable by the output driver. (Positions 1 through 4 are typically
// allocated to styles, and are not usable thus.) A return value of
-// `-1` indicates failure.
-// TODO: Make a `const int FONT_NOT_MOUNTED = -1;`, and use that in
-// callers (here and in "env.cpp"). Aping libc-style, `errno`-using
-// functions conceals more than it reveals here since we have no analog
-// to `errno`. (`errno` was a blind alley anyway, being as reentrant
-// and concurrency-friendly as a global lock. Option types,
-// error-storing function parameters, and exceptions are all better
-// solutions.)
+// `FONT_NOT_MOUNTED` indicates failure.
int font_family::resolve(int mounting_position)
{
assert(mounting_position >= 0);
int pos = mounting_position;
+ assert((pos >= 0) || (FONT_NOT_MOUNTED == pos));
if (pos < 0)
- return -1;
+ return FONT_NOT_MOUNTED;
if (pos < map_size && map[pos] >= 0)
return map[pos];
if (!((pos < font_table_size)
&& (font_table[pos] != 0 /* nullptr */)))
- return -1;
+ return FONT_NOT_MOUNTED;
if (pos >= map_size) {
int old_map_size = map_size;
int *old_map = map;
@@ -6639,7 +6635,7 @@ int font_family::resolve(int mounting_position)
memcpy(map, old_map, old_map_size * sizeof (int));
delete[] old_map;
for (int j = old_map_size; j < map_size; j++)
- map[j] = -1;
+ map[j] = FONT_NOT_MOUNTED;
}
if (!(font_table[pos]->is_style()))
return map[pos] = pos;
@@ -6655,7 +6651,7 @@ int font_family::resolve(int mounting_position)
if (n >= font_table_size) {
n = next_available_font_position();
if (!mount_font_no_translate(n, f, f))
- return -1;
+ return FONT_NOT_MOUNTED;
}
return map[pos] = n;
}
@@ -6681,10 +6677,10 @@ void font_family::invalidate_fontno(int n)
while (iter.get(&nam, (void **)&fam)) {
int mapsize = fam->map_size;
if (n < mapsize)
- fam->map[n] = -1;
+ fam->map[n] = FONT_NOT_MOUNTED;
for (int i = 0; i < mapsize; i++)
if (fam->map[i] == n)
- fam->map[i] = -1;
+ fam->map[i] = FONT_NOT_MOUNTED;
}
}
@@ -6726,8 +6722,7 @@ static void font_lookup_error(font_lookup_info& finfo,
// Read the next token and look it up as a font name or position number.
// Return lookup success. Store, in the supplied struct argument, the
-// requested name or position, and the position actually resolved; -1
-// means not found (see `font_lookup_info` constructor).
+// requested name or position, and the position actually resolved.
static bool has_font(font_lookup_info *finfo)
{
int n;
@@ -6752,7 +6747,7 @@ static bool has_font(font_lookup_info *finfo)
|| (0 /* nullptr */ == font_table[n])))
finfo->position = curenv->get_family()->resolve(n);
}
- return (finfo->position != -1);
+ return (finfo->position != FONT_NOT_MOUNTED);
}
static int underline_fontno = 2;
@@ -6959,7 +6954,7 @@ int symbol_fontno(symbol s)
if ((font_table[i] != 0 /* nullptr */)
&& font_table[i]->is_named(s))
return i;
- return -1;
+ return FONT_NOT_MOUNTED;
}
/* TODO: mark `inline`? */
diff --git a/src/roff/troff/node.h b/src/roff/troff/node.h
index 92c319986..62b301aee 100644
--- a/src/roff/troff/node.h
+++ b/src/roff/troff/node.h
@@ -740,6 +740,7 @@ font_family *lookup_family(symbol);
symbol get_font_name(int, environment *);
symbol get_style_name(int);
extern search_path include_search_path;
+extern const int FONT_NOT_MOUNTED;
// Local Variables:
// fill-column: 72
_______________________________________________
groff-commit mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/groff-commit
