From 08786bbb19bbe0fdf28adcbb305b02f54a237edf Mon Sep 17 00:00:00 2001 From: Matthew Fernandez Date: Thu, 22 Apr 2021 18:29:21 -0700 Subject: [PATCH] add sprintf-like helper The function sprintf is generally considered unsafe to use in modern C. In the Graphviz code base, its uses are slowly being replaced with safer alternatives (#1950). However, the alternative code is frequently overly verbose and counter-intuitive. This commit adds (hopefully) convenient alternative functions that can be used to replace sprintf uses. The gv_sprint function is similar to the GNU asprintf function with a different calling convention. The tests added alongside follow the same strategy as used for testing lib/vmalloc. --- lib/cgraph/Makefile.am | 4 +- lib/cgraph/cgraph.vcxproj | 2 + lib/cgraph/cgraph.vcxproj.filters | 6 ++ lib/cgraph/sprint.c | 66 +++++++++++++++++ lib/cgraph/sprint.h | 74 +++++++++++++++++++ lib/cgraph/test_sprint.c | 118 ++++++++++++++++++++++++++++++ rtest/test_sprint.py | 25 +++++++ 7 files changed, 293 insertions(+), 2 deletions(-) create mode 100644 lib/cgraph/sprint.c create mode 100644 lib/cgraph/sprint.h create mode 100644 lib/cgraph/test_sprint.c create mode 100644 rtest/test_sprint.py diff --git a/lib/cgraph/Makefile.am b/lib/cgraph/Makefile.am index aa06f9453..79c1c8625 100644 --- a/lib/cgraph/Makefile.am +++ b/lib/cgraph/Makefile.am @@ -8,7 +8,7 @@ pkgconfigdir = $(libdir)/pkgconfig AM_CPPFLAGS = -I$(top_srcdir)/lib -I$(top_srcdir)/lib/cdt pkginclude_HEADERS = cgraph.h -noinst_HEADERS = agxbuf.h cghdr.h strcasecmp.h +noinst_HEADERS = agxbuf.h cghdr.h sprint.h strcasecmp.h noinst_LTLIBRARIES = libcgraph_C.la lib_LTLIBRARIES = libcgraph.la pkgconfig_DATA = libcgraph.pc @@ -29,7 +29,7 @@ endif libcgraph_C_la_SOURCES = agerror.c agxbuf.c apply.c attr.c edge.c \ flatten.c graph.c grammar.y id.c imap.c io.c mem.c node.c \ - obj.c pend.c rec.c refstr.c scan.l subg.c utils.c write.c + obj.c pend.c rec.c refstr.c scan.l sprint.c subg.c utils.c write.c libcgraph_la_LDFLAGS = -version-info $(CGRAPH_VERSION) -no-undefined libcgraph_la_SOURCES = $(libcgraph_C_la_SOURCES) diff --git a/lib/cgraph/cgraph.vcxproj b/lib/cgraph/cgraph.vcxproj index 77a24fca4..4d1711460 100644 --- a/lib/cgraph/cgraph.vcxproj +++ b/lib/cgraph/cgraph.vcxproj @@ -101,6 +101,7 @@ win_flex -oscan.c scan.l + @@ -122,6 +123,7 @@ win_flex -oscan.c scan.l + diff --git a/lib/cgraph/cgraph.vcxproj.filters b/lib/cgraph/cgraph.vcxproj.filters index ae190d379..18099af33 100644 --- a/lib/cgraph/cgraph.vcxproj.filters +++ b/lib/cgraph/cgraph.vcxproj.filters @@ -24,6 +24,9 @@ Header Files + + Header Files + Header Files @@ -83,6 +86,9 @@ Source Files + + Source Files + Source Files diff --git a/lib/cgraph/sprint.c b/lib/cgraph/sprint.c new file mode 100644 index 000000000..6f9476d85 --- /dev/null +++ b/lib/cgraph/sprint.c @@ -0,0 +1,66 @@ +#include +#include +#include +#include +#include + +#ifdef __clang__ +#define NONNULL _Nonnull +#define NULLABLE _Nullable +#else +#define NONNULL /* nothing */ +#define NULLABLE /* nothing */ +#endif + +static char *NULLABLE print(const char *NONNULL format, va_list ap) { + assert(format != NULL); + + va_list ap2; + va_copy(ap2, ap); + + // how many bytes do we need to construct this string? + int r = vsnprintf(NULL, 0, format, ap2); + va_end(ap2); + if (r < 0) { + // bad format string + return NULL; + } + size_t len = (size_t)r + 1 /* for NUL */; + + // allocate space to construct the string + char *s = malloc(len); + if (s == NULL) { + return NULL; + } + + // construct it + (void)vsnprintf(s, len, format, ap); + + return s; +} + +char *NULLABLE gv_sprint(const char *NONNULL format, ...) { + assert(format != NULL); + + va_list ap; + va_start(ap, format); + char *s = print(format, ap); + va_end(ap); + return s; +} + +char *NONNULL gv_sprint_or_exit(const char *NONNULL format, ...) { + assert(format != NULL); + + va_list ap; + va_start(ap, format); + char *s = print(format, ap); + va_end(ap); + + if (s == NULL) { + fprintf(stderr, "gv_sprint failed\n"); + exit(EXIT_FAILURE); + } + + return s; +} diff --git a/lib/cgraph/sprint.h b/lib/cgraph/sprint.h new file mode 100644 index 000000000..def468c46 --- /dev/null +++ b/lib/cgraph/sprint.h @@ -0,0 +1,74 @@ +#pragma once + +#ifdef _WIN32 +#ifdef EXPORT_CGRAPH +#define DECLSPEC __declspec(dllexport) +#else +#define DECLSPEC __declspec(dllimport) +#endif +#else +#define DECLSPEC /* nothing */ +#endif + +#ifdef __clang__ +#define NONNULL _Nonnull +#define NULLABLE _Nullable +#else +#define NONNULL /* nothing */ +#define NULLABLE /* nothing */ +#endif + +#ifdef __GNUC__ +#define PRINTF_LIKE(fmt, args) __attribute__((format(printf, fmt, args))) +#else +#define PRINTF_LIKE(fmt, args) /* nothing */ +#endif + +#ifdef __GNUC__ +#if defined(__clang__) || __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 9) +#define RETURNS_NONNULL __attribute__((returns_nonnull)) +#else +#define RETURNS_NONNULL /* nothing */ +#endif +#else +#define RETURNS_NONNULL /* nothing */ +#endif + +#ifdef __GNUC__ +#define WUR __attribute__((warn_unused_result)) +#else +#define WUR +#endif + +/** sprintf-alike + * + * This function constructs a dynamically allocated string based on its + * printf-style arguments and returns a pointer to this string. It is intended + * to be used as a safe alternative to sprintf. + * + * @param format Printf-style format string + * @param ... Format arguments, if any + * @returns A pointer to the constructed string or NULL on failure + */ +DECLSPEC PRINTF_LIKE(1, 2) WUR +char *NULLABLE gv_sprint(const char *NONNULL format, ...); + +/** gv_sprint wrapper, calling exit if NULL is returned + * + * This alternative is provided for callers who have no reasonable way to handle + * a NULL return from gv_sprint. However, it is always preferable to call + * gv_sprint and handle the error gracefully for composability. + * + * @param format Printf-style format string + * @param ... Format arguments, if any + * @returns A pointer to the constructed string + */ +DECLSPEC PRINTF_LIKE(1, 2) RETURNS_NONNULL WUR +char *NONNULL gv_sprint_or_exit(const char *NONNULL format, ...); + +#undef WUR +#undef RETURNS_NONNULL +#undef PRINTF_LIKE +#undef NONNULL +#undef NULLABLE +#undef DECLSPEC diff --git a/lib/cgraph/test_sprint.c b/lib/cgraph/test_sprint.c new file mode 100644 index 000000000..532f6f886 --- /dev/null +++ b/lib/cgraph/test_sprint.c @@ -0,0 +1,118 @@ +// basic unit tester for gv_sprint + +#ifdef NDEBUG +#error this is not intended to be compiled with assertions off +#endif + +#include +#include +#include +#include +#include +#include + +// include sprint.c so we can be compiled standalone +#include + +// print "" +static void test_empty_string(void) { + char *s = gv_sprint(""); + assert(strcmp(s, "") == 0); + free(s); +} + +// print a basic string +static void test_simple(void) { + char *s = gv_sprint("hello world"); + assert(strcmp(s, "hello world") == 0); + free(s); +} + +// print something large +static void test_large(void) { + static const char text[] = + "Sed ut perspiciatis, unde omnis iste natus error sit voluptatem " + "accusantium doloremque laudantium, totam rem aperiam eaque ipsa, quae ab " + "illo inventore veritatis et quasi architecto beatae vitae dicta sunt, " + "explicabo. Nemo enim ipsam voluptatem, quia voluptas sit, aspernatur aut " + "odit aut fugit, sed quia consequuntur magni dolores eos, qui ratione " + "voluptatem sequi nesciunt, neque porro quisquam est, qui do lorem ipsum, " + "quia dolor sit amet consectetur adipisci[ng] velit, sed quia non numquam " + "[do] eius modi tempora inci[di]dunt, ut labore et dolore magnam aliquam " + "quaerat voluptatem. Ut enim ad minima veniam, quis nostrum[d] " + "exercitationem ullam corporis suscipit laboriosam, nisi ut aliquid ex ea " + "commodi consequatur? [D]Quis autem vel eum iure reprehenderit, qui in ea " + "voluptate velit esse, quam nihil molestiae consequatur, vel illum, qui " + "dolorem eum fugiat, quo voluptas nulla pariatur? [33] At vero eos et " + "accusamus et iusto odio dignissimos ducimus, qui blanditiis praesentium " + "voluptatum deleniti atque corrupti, quos dolores et quas molestias " + "excepturi sint, obcaecati cupiditate non provident, similique sunt in " + "culpa, qui officia deserunt mollitia animi, id est laborum et dolorum " + "fuga. Et harum quidem rerum facilis est et expedita distinctio. Nam " + "libero tempore, cum soluta nobis est eligendi optio, cumque nihil " + "impedit, quo minus id, quod maxime placeat, facere possimus, omnis " + "voluptas assumenda est, omnis dolor repellendus. Temporibus autem " + "quibusdam et aut officiis debitis aut rerum necessitatibus saepe eveniet, " + "ut et voluptates repudiandae sint et molestiae non recusandae. Itaque " + "earum rerum hic tenetur a sapiente delectus, ut aut reiciendis " + "voluptatibus maiores alias consequatur aut perferendis doloribus " + "asperiores repellat."; + char *s = gv_sprint(text); + assert(strcmp(s, text) == 0); + free(s); +} + +// print some non-ASCII text +static void test_utf8(void) { + static const char text[] = "Ça roule?"; + char *s = gv_sprint(text); + assert(strcmp(s, text) == 0); + free(s); +} + +// print various numbers +static void test_int(void) { + char *s = gv_sprint("%d is a nice number, more than %" PRId8 + ", less than %lu", 7, INT8_C(-1), 42ul); + assert(strcmp(s, "7 is a nice number, more than -1, less than 42") == 0); + free(s); +} + +// print some floating-point values +static void test_float(void) { + char *s = gv_sprint("%.4f as a double or %.4f as a float, " + "can be truncated to %.3f", 1.2345, 1.2345f, 1.2345); + assert(strcmp(s, "1.2345 as a double or 1.2345 as a float, " + "can be truncated to 1.234") == 0); + free(s); +} + +// escaping % +static void test_percent(void) { + char *s = gv_sprint("this is a percent: %%."); + assert(strcmp(s, "this is a percent: %.") == 0); + free(s); +} + +int main(void) { + +#define RUN(t) \ + do { \ + printf("running test_%s... ", #t); \ + fflush(stdout); \ + test_##t(); \ + printf("OK\n"); \ + } while (0) + + RUN(empty_string); + RUN(simple); + RUN(large); + RUN(utf8); + RUN(int); + RUN(float); + RUN(percent); + +#undef RUN + + return EXIT_SUCCESS; +} diff --git a/rtest/test_sprint.py b/rtest/test_sprint.py new file mode 100644 index 000000000..d1be6b420 --- /dev/null +++ b/rtest/test_sprint.py @@ -0,0 +1,25 @@ +"""test ../lib/cgraph/sprint.c""" + +import os +from pathlib import Path +import sys + +sys.path.append(os.path.dirname(__file__)) +from gvtest import run_c + +def test_sprint(): + """run the sprint unit tests""" + + # locate the sprint unit tests + src = Path(__file__).parent.resolve() / "../lib/cgraph/test_sprint.c" + assert src.exists() + + # locate lib directory that needs to be in the include path + lib = Path(__file__).parent.resolve() / "../lib" + + # extra C flags this compilation needs + cflags = ['-I', lib] + + ret, _, _ = run_c(src, cflags=cflags) + + assert ret == 0 -- 2.40.0