From 44e025bb4727be190f0ce0a2240f163f5f3d9732 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?= Date: Fri, 22 Jan 2021 07:42:45 +0700 Subject: [PATCH 1/6] writeANSI: cleanup code style Clean up all inconsistence code-style for the next refactor: - whitespace after keyword and before open brace - no whitespace after ( and before ) - whitespace around binary operator --- qrenc.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/qrenc.c b/qrenc.c index c09c4ab6..2a6a32cf 100644 --- a/qrenc.c +++ b/qrenc.c @@ -734,14 +734,14 @@ static int writeXPM(const QRcode *qrcode, const char *outfile) } static void writeANSI_margin(FILE* fp, int realwidth, - char* buffer, const char* white, int white_s ) + char* buffer, const char* white, int white_s) { int y; strncpy(buffer, white, (size_t)white_s); memset(buffer + white_s, ' ', (size_t)realwidth * 2); strcpy(buffer + white_s + realwidth * 2, "\033[0m\n"); // reset to default colors - for(y = 0; y < margin; y++ ){ + for (y = 0; y < margin; y++) { fputs(buffer, fp); } } @@ -758,7 +758,7 @@ static int writeANSI(const QRcode *qrcode, const char *outfile) char *buffer; int white_s, black_s, buffer_s; - if(image_type == ANSI256_TYPE){ + if (image_type == ANSI256_TYPE) { /* codes for 256 color compatible terminals */ white = "\033[48;5;231m"; white_s = 11; @@ -778,7 +778,7 @@ static int writeANSI(const QRcode *qrcode, const char *outfile) realwidth = (qrcode->width + margin * 2) * size; buffer_s = (realwidth * white_s) * 2; buffer = (char *)malloc((size_t)buffer_s); - if(buffer == NULL) { + if (buffer == NULL) { fprintf(stderr, "Failed to allocate memory.\n"); exit(EXIT_FAILURE); } @@ -788,33 +788,33 @@ static int writeANSI(const QRcode *qrcode, const char *outfile) /* data */ p = qrcode->data; - for(y = 0; y < qrcode->width; y++) { - row = (p+(y*qrcode->width)); + for (y = 0; y < qrcode->width; y++) { + row = p + (y * qrcode->width); memset(buffer, 0, (size_t)buffer_s); strncpy(buffer, white, (size_t)white_s); - for(x = 0; x < margin; x++ ){ + for (x = 0; x < margin; x++) { strncat(buffer, " ", 2); } last = 0; - for(x = 0; x < qrcode->width; x++) { - if(*(row+x)&0x1) { - if( last != 1 ){ + for (x = 0; x < qrcode->width; x++) { + if (row[x] & 0x1) { + if (last != 1) { strncat(buffer, black, (size_t)black_s); last = 1; } - } else if( last != 0 ){ + } else if(last != 0) { strncat(buffer, white, (size_t)white_s); last = 0; } strncat(buffer, " ", 2); } - if( last != 0 ){ + if (last != 0) { strncat(buffer, white, (size_t)white_s); } - for(x = 0; x < margin; x++ ){ + for (x = 0; x < margin; x++) { strncat(buffer, " ", 2); } strncat(buffer, "\033[0m\n", 5); From a3d7b1afc96131f40beead34f4681b98733b2008 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?= Date: Fri, 22 Jan 2021 07:42:45 +0700 Subject: [PATCH 2/6] writeANSI: use size_t for variable intended for size --- qrenc.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/qrenc.c b/qrenc.c index 2a6a32cf..113d78dc 100644 --- a/qrenc.c +++ b/qrenc.c @@ -733,13 +733,13 @@ static int writeXPM(const QRcode *qrcode, const char *outfile) return 0; } -static void writeANSI_margin(FILE* fp, int realwidth, - char* buffer, const char* white, int white_s) +static void writeANSI_margin(FILE* fp, size_t realwidth, + char* buffer, const char* white, size_t white_s) { int y; - strncpy(buffer, white, (size_t)white_s); - memset(buffer + white_s, ' ', (size_t)realwidth * 2); + strncpy(buffer, white, white_s); + memset(buffer + white_s, ' ', realwidth * 2); strcpy(buffer + white_s + realwidth * 2, "\033[0m\n"); // reset to default colors for (y = 0; y < margin; y++) { fputs(buffer, fp); @@ -756,7 +756,7 @@ static int writeANSI(const QRcode *qrcode, const char *outfile) const char *white, *black; char *buffer; - int white_s, black_s, buffer_s; + size_t white_s, black_s, buffer_s; if (image_type == ANSI256_TYPE) { /* codes for 256 color compatible terminals */ @@ -777,7 +777,7 @@ static int writeANSI(const QRcode *qrcode, const char *outfile) realwidth = (qrcode->width + margin * 2) * size; buffer_s = (realwidth * white_s) * 2; - buffer = (char *)malloc((size_t)buffer_s); + buffer = malloc(buffer_s); if (buffer == NULL) { fprintf(stderr, "Failed to allocate memory.\n"); exit(EXIT_FAILURE); @@ -791,8 +791,8 @@ static int writeANSI(const QRcode *qrcode, const char *outfile) for (y = 0; y < qrcode->width; y++) { row = p + (y * qrcode->width); - memset(buffer, 0, (size_t)buffer_s); - strncpy(buffer, white, (size_t)white_s); + memset(buffer, 0, buffer_s); + strncpy(buffer, white, white_s); for (x = 0; x < margin; x++) { strncat(buffer, " ", 2); } @@ -801,18 +801,18 @@ static int writeANSI(const QRcode *qrcode, const char *outfile) for (x = 0; x < qrcode->width; x++) { if (row[x] & 0x1) { if (last != 1) { - strncat(buffer, black, (size_t)black_s); + strncat(buffer, black, black_s); last = 1; } } else if(last != 0) { - strncat(buffer, white, (size_t)white_s); + strncat(buffer, white, white_s); last = 0; } strncat(buffer, " ", 2); } if (last != 0) { - strncat(buffer, white, (size_t)white_s); + strncat(buffer, white, white_s); } for (x = 0; x < margin; x++) { strncat(buffer, " ", 2); From fd9cb5446df626402cdf4ac69d4c21fe88546ca5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?= Date: Fri, 22 Jan 2021 07:42:45 +0700 Subject: [PATCH 3/6] writeANSI: use strlen to find length of string Modern compiler can optimise those calls, let's not burden ourselves with this counting. --- qrenc.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/qrenc.c b/qrenc.c index 113d78dc..33e1f7f9 100644 --- a/qrenc.c +++ b/qrenc.c @@ -761,15 +761,13 @@ static int writeANSI(const QRcode *qrcode, const char *outfile) if (image_type == ANSI256_TYPE) { /* codes for 256 color compatible terminals */ white = "\033[48;5;231m"; - white_s = 11; black = "\033[48;5;16m"; - black_s = 10; } else { white = "\033[47m"; - white_s = 5; black = "\033[40m"; - black_s = 5; } + white_s = strlen(white); + black_s = strlen(black); size = 1; From 6225427176d2c4093200b5cb51ea545db08176f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?= Date: Fri, 22 Jan 2021 07:42:45 +0700 Subject: [PATCH 4/6] writeANSI: allocate buffer based on longer color string On a modern compiler, the compiler should be able to figure out both black_s and white_s are compile time constant and fold this change to constant. And we can easier argue why we need to allow that much of memory. This change also help if we decide to support another kind of weird terminal color scheme in future. --- qrenc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qrenc.c b/qrenc.c index 33e1f7f9..ece9a1bc 100644 --- a/qrenc.c +++ b/qrenc.c @@ -774,7 +774,7 @@ static int writeANSI(const QRcode *qrcode, const char *outfile) fp = openFile(outfile); realwidth = (qrcode->width + margin * 2) * size; - buffer_s = (realwidth * white_s) * 2; + buffer_s = (realwidth * (white_s > black_s ? white_s : black_s)) * 2; buffer = malloc(buffer_s); if (buffer == NULL) { fprintf(stderr, "Failed to allocate memory.\n"); From 176de82eb60b6b17a5dc659a78c431aa00cb137b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?= Date: Fri, 22 Jan 2021 07:42:45 +0700 Subject: [PATCH 5/6] writeANSI: use strcpy instead of strncat strncat(3) needs to traverse through the buffer to append the string into the end of old string. This will have a big performance penalty when we have a very large input. Replace it with strcpy(3) and a pointer to track the end of current buffer. Accidently, this change also silence gcc's -Wstringop-overflow warning, since gcc thinks our issue to strncat(3) to stop right at the NULL terminator maybe an error. --- qrenc.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/qrenc.c b/qrenc.c index ece9a1bc..17adc72e 100644 --- a/qrenc.c +++ b/qrenc.c @@ -746,6 +746,13 @@ static void writeANSI_margin(FILE* fp, size_t realwidth, } } +#define COPY_AND_ADVANCE(BUF, STR) \ + do { \ + strcpy(BUF, STR); \ + BUF += strlen(STR); \ + } while (0) + + static int writeANSI(const QRcode *qrcode, const char *outfile) { FILE *fp; @@ -756,6 +763,7 @@ static int writeANSI(const QRcode *qrcode, const char *outfile) const char *white, *black; char *buffer; + char *pbuf; size_t white_s, black_s, buffer_s; if (image_type == ANSI256_TYPE) { @@ -790,32 +798,33 @@ static int writeANSI(const QRcode *qrcode, const char *outfile) row = p + (y * qrcode->width); memset(buffer, 0, buffer_s); - strncpy(buffer, white, white_s); + pbuf = buffer; + COPY_AND_ADVANCE(pbuf, white); for (x = 0; x < margin; x++) { - strncat(buffer, " ", 2); + COPY_AND_ADVANCE(pbuf, " "); } last = 0; for (x = 0; x < qrcode->width; x++) { if (row[x] & 0x1) { if (last != 1) { - strncat(buffer, black, black_s); + COPY_AND_ADVANCE(pbuf, black); last = 1; } } else if(last != 0) { - strncat(buffer, white, white_s); + COPY_AND_ADVANCE(pbuf, white); last = 0; } - strncat(buffer, " ", 2); + COPY_AND_ADVANCE(pbuf, " "); } if (last != 0) { - strncat(buffer, white, white_s); + COPY_AND_ADVANCE(pbuf, white); } for (x = 0; x < margin; x++) { - strncat(buffer, " ", 2); + COPY_AND_ADVANCE(pbuf, " "); } - strncat(buffer, "\033[0m\n", 5); + COPY_AND_ADVANCE(pbuf, "\033[0m\n"); fputs(buffer, fp); } From 461d4e8e2681a9009f014b1f4708e4e8f425f8c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?= Date: Fri, 22 Jan 2021 07:42:46 +0700 Subject: [PATCH 6/6] writeANSI, writeASCII: do not reset size We're reseting size to 1 in writeANSI and writeASCII, this may be problematic if someone use qrencode as a library. Let's just ignore it in ASCII/ANSI mode instead. --- qrenc.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/qrenc.c b/qrenc.c index 17adc72e..a937d7d9 100644 --- a/qrenc.c +++ b/qrenc.c @@ -777,11 +777,9 @@ static int writeANSI(const QRcode *qrcode, const char *outfile) white_s = strlen(white); black_s = strlen(black); - size = 1; - fp = openFile(outfile); - realwidth = (qrcode->width + margin * 2) * size; + realwidth = qrcode->width + margin * 2; buffer_s = (realwidth * (white_s > black_s ? white_s : black_s)) * 2; buffer = malloc(buffer_s); if (buffer == NULL) { @@ -966,8 +964,6 @@ static int writeASCII(const QRcode *qrcode, const char *outfile, int invert) white = '#'; } - size = 1; - fp = openFile(outfile); realwidth = (qrcode->width + margin * 2) * 2;