From b1101995a29dce19267d1dbd9b0db51366f82708 Mon Sep 17 00:00:00 2001 From: Matthew Fernandez Date: Sat, 9 Oct 2021 09:27:54 -0700 Subject: [PATCH 1/7] xml_isentity: return a proper bool instead of using an int as a bool --- lib/common/xml.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/common/xml.c b/lib/common/xml.c index 9a246447e9..18731015b8 100644 --- a/lib/common/xml.c +++ b/lib/common/xml.c @@ -16,11 +16,11 @@ static bool isalpha_no_locale(char c) { * or &#[0-9]*; (e.g. & ) * or &#x[0-9a-fA-F]*; (e.g. 水 ) */ -static int xml_isentity(const char *s) +static bool xml_isentity(const char *s) { s++; /* already known to be '&' */ if (*s == ';') { // '&;' is not a valid entity - return 0; + return false; } if (*s == '#') { s++; @@ -37,8 +37,8 @@ static int xml_isentity(const char *s) s++; } if (*s == ';') - return 1; - return 0; + return true; + return false; } /** XML-escape a character -- GitLab From fa4765dd140ec92e52beb5095401c534dfc2c5ff Mon Sep 17 00:00:00 2001 From: Matthew Fernandez Date: Sat, 9 Oct 2021 09:44:25 -0700 Subject: [PATCH 2/7] gv2gxl: rename 'Agnodeinfo_t' type to 'Local_Agnodeinfo_t' This has no immediate effect, but having a type named `Agnodeinfo_t` prevents including lib/common/types.h which has its own `Agnodeinfo_t`. We will want to include this header (transitively) in an upcoming commit. Related to #1868. --- cmd/tools/gv2gxl.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/tools/gv2gxl.c b/cmd/tools/gv2gxl.c index c19b6c8076..1bf55ab216 100644 --- a/cmd/tools/gv2gxl.c +++ b/cmd/tools/gv2gxl.c @@ -41,7 +41,7 @@ typedef struct { Agrec_t h; int written; -} Agnodeinfo_t; +} Local_Agnodeinfo_t; static int Level; /* level of tabs */ static Agsym_t *Tailport, *Headport; @@ -800,7 +800,7 @@ writeEdge(gxlstate_t * stp, Agedge_t * e, FILE * gxlFile, Dict_t * d) } -#define writeval(n) (((Agnodeinfo_t*)((n)->base.data))->written) +#define writeval(n) (((Local_Agnodeinfo_t*)((n)->base.data))->written) static void writeBody(gxlstate_t * stp, Agraph_t * g, FILE * gxlFile) { @@ -909,7 +909,7 @@ static void freeState(gxlstate_t * stp) void gv_to_gxl(Agraph_t * g, FILE * gxlFile) { gxlstate_t *stp = initState(g); - aginit(g, AGNODE, "node", sizeof(Agnodeinfo_t), TRUE); + aginit(g, AGNODE, "node", sizeof(Local_Agnodeinfo_t), TRUE); iterateHdr(stp, g); iterateBody(stp, g); -- GitLab From 0d3f701e428e6c99d670069006faedd3534256d7 Mon Sep 17 00:00:00 2001 From: Matthew Fernandez Date: Sat, 9 Oct 2021 11:16:10 -0700 Subject: [PATCH 3/7] gxl2gv: realign MSVC include paths with the Autotools build system --- cmd/tools/gxl2gv.vcxproj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/tools/gxl2gv.vcxproj b/cmd/tools/gxl2gv.vcxproj index 84075525c0..835296c36e 100644 --- a/cmd/tools/gxl2gv.vcxproj +++ b/cmd/tools/gxl2gv.vcxproj @@ -53,7 +53,7 @@ Disabled - $(SolutionDir)windows\include;$(SolutionDir)windows\dependencies\libraries\vcpkg\installed\x86-windows\include;$(SolutionDir)windows\dependencies\libraries\x86\include;$(SolutionDir);$(SolutionDir)lib;$(SolutionDir)lib\cdt;$(SolutionDir)lib\cgraph;%(AdditionalIncludeDirectories) + $(SolutionDir)windows\include;$(SolutionDir)windows\dependencies\libraries\vcpkg\installed\x86-windows\include;$(SolutionDir)windows\dependencies\libraries\x86\include;$(SolutionDir);$(SolutionDir)lib;$(SolutionDir)lib\cdt;$(SolutionDir)lib\cgraph;$(SolutionDir)lib\pathplan;$(SolutionDir)lib\pack;$(SolutionDir)lib\gvc;$(SolutionDir)lib\common;%(AdditionalIncludeDirectories) _DEBUG;_CONSOLE;WIN32_DLL;%(PreprocessorDefinitions) EnableFastChecks MultiThreadedDebugDLL @@ -78,7 +78,7 @@ copy $(SolutionDir)windows\dependencies\libraries\vcpkg\installed\x86-windows\bi - $(SolutionDir)windows\include;$(SolutionDir)windows\dependencies\libraries\vcpkg\installed\x86-windows\include;$(SolutionDir)windows\dependencies\libraries\x86\include;$(SolutionDir);$(SolutionDir)lib;$(SolutionDir)lib\cdt;$(SolutionDir)lib\cgraph;%(AdditionalIncludeDirectories) + $(SolutionDir)windows\include;$(SolutionDir)windows\dependencies\libraries\vcpkg\installed\x86-windows\include;$(SolutionDir)windows\dependencies\libraries\x86\include;$(SolutionDir);$(SolutionDir)lib;$(SolutionDir)lib\cdt;$(SolutionDir)lib\cgraph;$(SolutionDir)lib\pathplan;$(SolutionDir)lib\pack;$(SolutionDir)lib\gvc;$(SolutionDir)lib\common;%(AdditionalIncludeDirectories) NDEBUG;_CONSOLE;%(PreprocessorDefinitions) Level4 -- GitLab From e7f323783fe2db9e722970c397ff8e75a270ffc9 Mon Sep 17 00:00:00 2001 From: Matthew Fernandez Date: Sat, 9 Oct 2021 09:55:44 -0700 Subject: [PATCH 4/7] remove legacy Makefile comment about missing gv2gxl.vcxproj gv2gxl is built by gxl2gv.vcxproj, that does exist. --- cmd/tools/Makefile.am | 3 --- 1 file changed, 3 deletions(-) diff --git a/cmd/tools/Makefile.am b/cmd/tools/Makefile.am index 5a293619b3..9990133376 100644 --- a/cmd/tools/Makefile.am +++ b/cmd/tools/Makefile.am @@ -387,9 +387,6 @@ EXTRA_DIST = $(man_MANS) $(pdf) bcomps.vcxproj* \ gmlscan.c gmlparse.c gmlparse.h gml2gv.vcxproj* \ graphml2gv.vcxproj* gv2gml.vcxproj* -# FIXME - these are missing -# gv2gxl.vcxproj* - CLEANFILES = stamp.h DISTCLEANFILES = $(pdf) gmlparse.[ch] gmlscan.c \ -- GitLab From cb15d4a48e2171f62fde320686a89689ee32d403 Mon Sep 17 00:00:00 2001 From: Matthew Fernandez Date: Sat, 9 Oct 2021 09:58:21 -0700 Subject: [PATCH 5/7] gv2gxl: also link to libgvc Upcoming changes will make use of some functionality from libcommon, which is consolidated into libgvc. Note that this commit contains no changes to gxl2gv.vcxproj because it already lists libgvc as a library to link against. Related to #1868. --- cmd/tools/CMakeLists.txt | 1 + cmd/tools/Makefile.am | 1 + 2 files changed, 2 insertions(+) diff --git a/cmd/tools/CMakeLists.txt b/cmd/tools/CMakeLists.txt index 412a386943..94e79b8e07 100644 --- a/cmd/tools/CMakeLists.txt +++ b/cmd/tools/CMakeLists.txt @@ -294,6 +294,7 @@ target_include_directories(gxl2gv PRIVATE target_link_libraries(gxl2gv cgraph + gvc ingraphs ${EXPAT_LIBRARIES} ) diff --git a/cmd/tools/Makefile.am b/cmd/tools/Makefile.am index 9990133376..4978feafbe 100644 --- a/cmd/tools/Makefile.am +++ b/cmd/tools/Makefile.am @@ -74,6 +74,7 @@ endif gxl2gv_SOURCES = cvtgxl.c gv2gxl.c gxl2gv.c gxl2gv_LDADD = \ + $(top_builddir)/lib/gvc/libgvc.la \ $(top_builddir)/lib/ingraphs/libingraphs_C.la \ $(top_builddir)/lib/cgraph/libcgraph.la \ $(top_builddir)/lib/cdt/libcdt.la @EXPAT_LIBS@ -- GitLab From 3b6635e045ff5d48f6a6a587fbfc2bfe45b23a4b Mon Sep 17 00:00:00 2001 From: Matthew Fernandez Date: Sat, 9 Oct 2021 10:16:08 -0700 Subject: [PATCH 6/7] =?UTF-8?q?gv2gxl:=20replace=20'fprintf(=E2=80=A6=20xm?= =?UTF-8?q?l=5Furl=5Fstring(=E2=80=A6))'=20with=20libcommon=E2=80=99s=20'x?= =?UTF-8?q?ml=5Fescape(=E2=80=A6)'?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes the affected code thread-safe (not so relevant as gv2gxl is single threaded) and removes unnecessary dynamic memory allocation. However, the main motivation for this is de-duping the XML-escaping functionality in Graphviz. Related to #1868. --- cmd/tools/gv2gxl.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/cmd/tools/gv2gxl.c b/cmd/tools/gv2gxl.c index 1bf55ab216..a3e698a3cb 100644 --- a/cmd/tools/gv2gxl.c +++ b/cmd/tools/gv2gxl.c @@ -8,9 +8,11 @@ * Contributors: Details at https://graphviz.org *************************************************************************/ - +#include +#include #include "convert.h" #include +#include #include #define SMALLBUF 128 @@ -251,9 +253,16 @@ static char *xml_string(char *s) return _xml_string(s,1); } -static char *xml_url_string(char *s) -{ - return _xml_string(s,0); +// `fputs` wrapper to handle the difference in calling convention to what +// `xml_escape`’s `cb` expects +static inline int put(void *stream, const char *s) { + return fputs(s, stream); +} + +// wrapper around `xml_escape` to set flags for URL escaping +static int xml_url_puts(FILE *f, const char *s) { + const xml_flags_t flags = {0}; + return xml_escape(s, flags, put, f); } static int isGxlGrammar(char *name) @@ -407,7 +416,9 @@ static void printHref(FILE * gxlFile, void *n) val = agget(n, GXL_TYPE); if (!EMPTY(val)) { tabover(gxlFile); - fprintf(gxlFile, "\t\n", xml_url_string(val)); + fprintf(gxlFile, "\t\n"); tabover(gxlFile); fprintf(gxlFile, "\t\n"); } @@ -443,8 +454,9 @@ writeDict(Agraph_t * g, FILE * gxlFile, char *name, Dict_t * dict, tabover(gxlFile); fprintf(gxlFile, "\t\n", xml_string(sym->name)); tabover(gxlFile); - fprintf(gxlFile, "\t\t\n", - xml_url_string(locatorVal)); + fprintf(gxlFile, "\t\t\n"); tabover(gxlFile); fprintf(gxlFile, "\t\n"); } else { @@ -653,9 +665,9 @@ writeNondefaultAttr(void *obj, FILE * gxlFile, Dict_t * defdict) fprintf(gxlFile, "\t\n", xml_string(sym->name)); tabover(gxlFile); - fprintf(gxlFile, - "\t\t\n", - xml_url_string(locatorVal)); + fprintf(gxlFile, "\t\t\n"); tabover(gxlFile); fprintf(gxlFile, "\t\n"); } else { -- GitLab From 269f02283d2459446ebc2926df2f82b4e765c394 Mon Sep 17 00:00:00 2001 From: Matthew Fernandez Date: Sat, 9 Oct 2021 10:45:05 -0700 Subject: [PATCH 7/7] =?UTF-8?q?gv2gxl:=20replace=20'fprintf(=E2=80=A6=20xm?= =?UTF-8?q?l=5Fstring(=E2=80=A6))'=20with=20libcommon=E2=80=99s=20'xml=5Fe?= =?UTF-8?q?scape(=E2=80=A6)'?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes the affected code thread-safe (not so relevant as gv2gxl is single threaded) and removes unnecessary dynamic memory allocation. However, the main motivation for this is de-duping the XML-escaping functionality in Graphviz. Related to #1868. --- cmd/tools/gv2gxl.c | 209 +++++++++++++++++---------------------------- 1 file changed, 79 insertions(+), 130 deletions(-) diff --git a/cmd/tools/gv2gxl.c b/cmd/tools/gv2gxl.c index a3e698a3cb..69f59e27af 100644 --- a/cmd/tools/gv2gxl.c +++ b/cmd/tools/gv2gxl.c @@ -150,115 +150,18 @@ static int legalGXLName(char *id) return 1; } -/* return true if *s points to &[A-Za-z]+; (e.g. Ç ) - * or &#[0-9]*; (e.g. & ) - * or &#x[0-9a-fA-F]*; (e.g. 水 ) - */ -static int xml_isentity(char *s) -{ - s++; /* already known to be '&' */ - if (*s == ';') { // '&;' is not a valid entity - return 0; - } - if (*s == '#') { - s++; - if (*s == 'x' || *s == 'X') { - s++; - while ((*s >= '0' && *s <= '9') - || (*s >= 'a' && *s <= 'f') - || (*s >= 'A' && *s <= 'F')) - s++; - } else { - while (*s >= '0' && *s <= '9') - s++; - } - } else { - while ((*s >= 'a' && *s <= 'z') - || (*s >= 'A' && *s <= 'Z')) - s++; - } - if (*s == ';') - return 1; - return 0; -} - -static char *_xml_string(char *s, int notURL) -{ - static char *buf = NULL; - static size_t bufsize = 0; - char *p, *sub, *prev = NULL; - size_t len, pos = 0; - - if (!buf) { - bufsize = 64; - buf = N_NEW(bufsize, char); - } - - p = buf; - while (s && *s) { - if (pos > (bufsize - 8)) { - bufsize *= 2; - buf = realloc(buf, bufsize); - p = buf + pos; - } - /* escape '&' only if not part of a legal entity sequence */ - if (*s == '&' && !(xml_isentity(s))) { - sub = "&"; - len = 5; - } - /* '<' '>' are safe to substitute even if string is already UTF-8 coded - * since UTF-8 strings won't contain '<' or '>' */ - else if (*s == '<') { - sub = "<"; - len = 4; - } - else if (*s == '>') { - sub = ">"; - len = 4; - } - else if ((*s == '-') && notURL) { /* can't be used in xml comment strings */ - sub = "-"; - len = 5; - } - else if (*s == ' ' && prev && *prev == ' ' && notURL) { - /* substitute 2nd and subsequent spaces with required_spaces */ - sub = " "; /* inkscape doesn't recognise   */ - len = 6; - } - else if (*s == '"') { - sub = """; - len = 6; - } - else if (*s == '\'') { - sub = "'"; - len = 5; - } - else { - sub = s; - len = 1; - } - while (len--) { - *p++ = *sub++; - pos++; - } - prev = s; - s++; - } - *p = '\0'; - return buf; -} - -static char *xml_string(char *s) -{ - return _xml_string(s,1); -} - // `fputs` wrapper to handle the difference in calling convention to what // `xml_escape`’s `cb` expects static inline int put(void *stream, const char *s) { return fputs(s, stream); } +// write a string to the given file, XML-escaping the input +static inline int xml_puts(FILE *stream, const char *s) { + const xml_flags_t flags = {.dash = 1, .nbsp = 1}; + return xml_escape(s, flags, put, stream); +} + // wrapper around `xml_escape` to set flags for URL escaping static int xml_url_puts(FILE *f, const char *s) { const xml_flags_t flags = {0}; @@ -382,11 +285,15 @@ static void graphAttrs(FILE * gxlFile, Agraph_t * g) val = agget(g, GXL_ROLE); if (!EMPTY(val)) { - fprintf(gxlFile, " role=\"%s\"", xml_string(val)); + fprintf(gxlFile, " role=\""); + xml_puts(gxlFile, val); + fprintf(gxlFile, "\""); } val = agget(g, GXL_HYPER); if (!EMPTY(val)) { - fprintf(gxlFile, " hypergraph=\"%s\"", xml_string(val)); + fprintf(gxlFile, " hypergraph=\""); + xml_puts(gxlFile, val); + fprintf(gxlFile, "\""); } } @@ -396,15 +303,21 @@ static void edgeAttrs(FILE * gxlFile, Agedge_t * e) val = agget(e, GXL_ID); if (!EMPTY(val)) { - fprintf(gxlFile, " id=\"%s\"", xml_string(val)); + fprintf(gxlFile, " id=\""); + xml_puts(gxlFile, val); + fprintf(gxlFile, "\""); } val = agget(e, GXL_FROM); if (!EMPTY(val)) { - fprintf(gxlFile, " fromorder=\"%s\"", xml_string(val)); + fprintf(gxlFile, " fromorder=\""); + xml_puts(gxlFile, val); + fprintf(gxlFile, "\""); } val = agget(e, GXL_TO); if (!EMPTY(val)) { - fprintf(gxlFile, " toorder=\"%s\"", xml_string(val)); + fprintf(gxlFile, " toorder=\""); + xml_puts(gxlFile, val); + fprintf(gxlFile, "\""); } } @@ -452,7 +365,9 @@ writeDict(Agraph_t * g, FILE * gxlFile, char *name, Dict_t * dict, locatorVal += 13; tabover(gxlFile); - fprintf(gxlFile, "\t\n", xml_string(sym->name)); + fprintf(gxlFile, "\tname); + fprintf(gxlFile, "\">\n"); tabover(gxlFile); fprintf(gxlFile, "\t\tname)); - fprintf(gxlFile, "kind=\"%s\">\n", xml_string(name)); + fprintf(gxlFile, "\tname); + fprintf(gxlFile, "\" "); + fprintf(gxlFile, "kind=\""); + xml_puts(gxlFile, name); + fprintf(gxlFile, "\">\n"); } else { - fprintf(gxlFile, "\tname)); - fprintf(gxlFile, "%s\">\n", xml_string(name)); + fprintf(gxlFile, "\tname); + fprintf(gxlFile, "\" kind=\""); + xml_puts(gxlFile, name); + fprintf(gxlFile, "\">\n"); } tabover(gxlFile); - fprintf(gxlFile, "\t\t%s\n", xml_string(sym->defval)); + fprintf(gxlFile, "\t\t"); + xml_puts(gxlFile, sym->defval); + fprintf(gxlFile, "\n"); tabover(gxlFile); fprintf(gxlFile, "\t\n"); } @@ -487,10 +412,16 @@ writeDict(Agraph_t * g, FILE * gxlFile, char *name, Dict_t * dict, } tabover(gxlFile); - fprintf(gxlFile, "\tname) + GXL_COMP_LEN))); - fprintf(gxlFile, "kind=\"%s\">\n", xml_string(name)); + fprintf(gxlFile, "\tname + GXL_COMP_LEN); + fprintf(gxlFile, "\" "); + fprintf(gxlFile, "kind=\""); + xml_puts(gxlFile, name); + fprintf(gxlFile, "\">\n"); tabover(gxlFile); - fprintf(gxlFile, "\t\t%s\n", xml_string(sym->defval)); + fprintf(gxlFile, "\t\t"); + xml_puts(gxlFile, sym->defval); + fprintf(gxlFile, "\n"); tabover(gxlFile); fprintf(gxlFile, "\t\n"); } @@ -566,7 +497,9 @@ writeHdr(gxlstate_t * stp, Agraph_t * g, FILE * gxlFile, int top) tabover(gxlFile); fprintf(gxlFile, "\t\n"); tabover(gxlFile); - fprintf(gxlFile, "\t\t%s\n", xml_string(name)); + fprintf(gxlFile, "\t\t"); + xml_puts(gxlFile, name); + fprintf(gxlFile, "\n"); tabover(gxlFile); fprintf(gxlFile, "\t\n"); } @@ -619,7 +552,9 @@ static int writeEdgeName(Agedge_t * e, FILE * gxlFile) tabover(gxlFile); fprintf(gxlFile, "\t\n"); tabover(gxlFile); - fprintf(gxlFile, "\t\t%s\n", xml_string(p)); + fprintf(gxlFile, "\t\t"); + xml_puts(gxlFile, p); + fprintf(gxlFile, "\n"); tabover(gxlFile); fprintf(gxlFile, "\t\n"); rv = TRUE; @@ -662,8 +597,9 @@ writeNondefaultAttr(void *obj, FILE * gxlFile, Dict_t * defdict) locatorVal += 13; tabover(gxlFile); - fprintf(gxlFile, "\t\n", - xml_string(sym->name)); + fprintf(gxlFile, "\tname); + fprintf(gxlFile, "\">\n"); tabover(gxlFile); fprintf(gxlFile, "\t\t\n"); } else { tabover(gxlFile); - fprintf(gxlFile, "\tname)); + fprintf(gxlFile, "\tname); + fprintf(gxlFile, "\""); if (aghtmlstr(data->str[sym->id])) { // This is a <…> string. Note this in the kind. fprintf(gxlFile, " kind=\"HTML-like string\""); } fprintf(gxlFile, ">\n"); tabover(gxlFile); - fprintf(gxlFile, "\t\t%s\n", xml_string(data->str[sym->id])); + fprintf(gxlFile, "\t\t"); + xml_puts(gxlFile, data->str[sym->id]); + fprintf(gxlFile, "\n"); tabover(gxlFile); fprintf(gxlFile, "\t\n"); } @@ -690,10 +630,13 @@ writeNondefaultAttr(void *obj, FILE * gxlFile, Dict_t * defdict) if (data->str[sym->id] != sym->defval) { tabover(gxlFile); - fprintf(gxlFile, "\t\n", - xml_string(((sym->name) + GXL_COMP_LEN))); + fprintf(gxlFile, "\tname + GXL_COMP_LEN); + fprintf(gxlFile, "\">\n"); tabover(gxlFile); - fprintf(gxlFile, "\t\t%s\n", xml_string(data->str[sym->id])); + fprintf(gxlFile, "\t\t"); + xml_puts(gxlFile, data->str[sym->id]); + fprintf(gxlFile, "\n"); tabover(gxlFile); fprintf(gxlFile, "\t\n"); } @@ -729,7 +672,9 @@ writeNode(gxlstate_t * stp, Agnode_t * n, FILE * gxlFile, Dict_t * d) tabover(gxlFile); fprintf(gxlFile, "\t\n"); tabover(gxlFile); - fprintf(gxlFile, "\t\t%s\n", xml_string(name)); + fprintf(gxlFile, "\t\t"); + xml_puts(gxlFile, name); + fprintf(gxlFile, "\n"); tabover(gxlFile); fprintf(gxlFile, "\t\n"); } @@ -748,9 +693,13 @@ static void writePort(Agedge_t * e, FILE * gxlFile, char *name) val = agget(e, name); if (val && val[0]) { tabover(gxlFile); - fprintf(gxlFile, "\t\n", xml_string(name)); + fprintf(gxlFile, "\t\n"); tabover(gxlFile); - fprintf(gxlFile, "\t\t%s\n", xml_string(val)); + fprintf(gxlFile, "\t\t"); + xml_puts(gxlFile, val); + fprintf(gxlFile, "\n"); tabover(gxlFile); fprintf(gxlFile, "\t\n"); } -- GitLab