Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch updating Makefile to -std=c11, including <inttypes.h>, and resolving conversion specifier warnings. #54

Closed
drankinatty opened this issue Jan 8, 2019 · 6 comments

Comments

@drankinatty
Copy link

Below is a short patch that resolves numerous warnings and updates the makefile to compile to the C standard that is only 8-years old as opposed to 30-years old. It also resolves the cast of int64_t values and the use of %lld by including inttypes.h and using the proper PRId64 conversion specifier. Additionally, warnings regarding signed and unsigned values used within a conditional expression are resolved via a cast to size_t.

@drankinatty
Copy link
Author

For some reason github didn't upload the file with the .patch extension so I'll try again with .txt, if this doesn't work, I'll just paste the patch. Github has gotten very flakey lately.

@drankinatty
Copy link
Author

Frustrating, 3rd try pasting:

diff -uNb p/sort-master/Makefile sort-master/Makefile
--- p/sort-master/Makefile	2018-12-27 16:54:06.000000000 -0600
+++ sort-master/Makefile	2019-01-08 13:17:29.200233924 -0600
@@ -2,7 +2,7 @@
 # Copyright (c) 2012 Google Inc. All Rights Reserved.
 
 CC ?= gcc
-CFLAGS = -O3 -g -Wall -std=c89 -pedantic
+CFLAGS = -Wall -Wextra -pedantic -Wshadow -std=c11 -Ofast
 
 default: demo stresstest multidemo test
 
diff -uNb p/sort-master/benchmark.c sort-master/benchmark.c
--- p/sort-master/benchmark.c	2018-12-27 16:54:06.000000000 -0600
+++ sort-master/benchmark.c	2019-01-08 12:10:48.922016426 -0600
@@ -96,7 +96,7 @@
       } \
     } \
     free(dst); \
-    printf("Benchmark%s%lld_%s\t%d\t%.1f ns/op\n", capital_word, size, platform, iter, diff * 1000.0 / (double) iter); \
+    printf("Benchmark%-24s%" PRId64 "_%s\t%4d\t%12.1f ns/op\n", capital_word, size, platform, iter, diff * 1000.0 / (double) iter); \
   } \
 } while (0)
 
@@ -119,7 +119,7 @@
       } \
     } \
     free(dst); \
-    printf("Benchmark%s%lld_%s\t%d\t%.1f ns/op\n", capital_word, size, platform, iter, diff * 1000.0 / (double) iter); \
+    printf("Benchmark%-24s%" PRId64 "_%s\t%4d\t%12.1f ns/op\n", capital_word, size, platform, iter, diff * 1000.0 / (double) iter); \
   } \
 } while (0)
 
diff -uNb p/sort-master/benchmark.txt sort-master/benchmark.txt
--- p/sort-master/benchmark.txt	1969-12-31 18:00:00.000000000 -0600
+++ sort-master/benchmark.txt	2019-01-08 12:11:06.458618906 -0600
@@ -0,0 +1,12 @@
+BenchmarkQsort                   100000_x86_64	  83	  12183036.1 ns/op
+BenchmarkBinary_insertion_sort   100000_x86_64	   1	1510590000.0 ns/op
+BenchmarkQuick_sort              100000_x86_64	 143	   7025923.1 ns/op
+BenchmarkMerge_sort              100000_x86_64	 121	   8331884.3 ns/op
+BenchmarkHeap_sort               100000_x86_64	 104	   9684769.2 ns/op
+BenchmarkShell_sort              100000_x86_64	  97	  10407948.5 ns/op
+BenchmarkTim_sort                100000_x86_64	 115	   8698547.8 ns/op
+BenchmarkMerge_sort_in_place     100000_x86_64	 136	   7401617.6 ns/op
+BenchmarkGrail_sort              100000_x86_64	  97	  10410886.6 ns/op
+BenchmarkSqrt_sort               100000_x86_64	 113	   8891053.1 ns/op
+BenchmarkRec_stable_sort         100000_x86_64	  34	  29496852.9 ns/op
+BenchmarkGrail_sort_dyn_buffer   100000_x86_64	  99	  10106505.1 ns/op
diff -uNb p/sort-master/demo.c sort-master/demo.c
--- p/sort-master/demo.c	2018-12-27 16:54:06.000000000 -0600
+++ sort-master/demo.c	2019-01-08 11:55:29.698447579 -0600
@@ -42,7 +42,7 @@
       printf("Verify failed! at %d\n", i);
 
       for (i = i - 2; i < SIZE; i++) {
-        printf(" %lld", (long long int)dst[i]);
+        printf(" %" PRId64 , dst[i]);
       }
 
       printf("\n");
Common subdirectories: p/sort-master/doc and sort-master/doc
diff -uNb p/sort-master/multidemo.c sort-master/multidemo.c
--- p/sort-master/multidemo.c	2018-12-27 16:54:06.000000000 -0600
+++ sort-master/multidemo.c	2019-01-08 11:55:32.574543834 -0600
@@ -71,7 +71,7 @@
       printf("Verify failed! at %d\n", i);
 
       for (i = i - 2; i < SIZE; i++) {
-        printf(" %lld", (long long int)dst[i]);
+        printf(" %" PRId64, dst[i]);
       }
 
       printf("\n");
@@ -88,7 +88,7 @@
       printf("Verify failed! at %d\n", i);
 
       for (i = i - 2; i < SIZE; i++) {
-        printf(" %lld", (long long int)dst[i]);
+        printf(" %" PRId64, dst[i]);
       }
 
       printf("\n");
diff -uNb p/sort-master/sort.h sort-master/sort.h
--- p/sort-master/sort.h	2018-12-27 16:54:06.000000000 -0600
+++ sort-master/sort.h	2019-01-08 12:02:19.288495210 -0600
@@ -6,6 +6,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <stdint.h>
+#include <inttypes.h>
 
 #ifndef SORT_NAME
 #error "Must declare SORT_NAME"
@@ -1146,7 +1147,7 @@
   size_t A_start = stack[stack_curr - 2].start;
   size_t B_start = stack[stack_curr - 1].start;
   SORT_TYPE *storage;
-  size_t i, j, k;
+  size_t k;
   /* A[k-1] <= B[0] < A[k] */
   k = TIM_SORT_GALLOP(&dst[A_start], A, dst[B_start], 0, 1);
   A_start += k;
@@ -1709,7 +1710,7 @@
   SORT_TYPE *ExtBuf;
   int *Tags;
 
-  while (L * L < Len) {
+  while ((size_t)(L * L) < Len) {
     L *= 2;
   }
 
@@ -2451,7 +2452,7 @@
   int L = 1;
   SORT_TYPE *ExtBuf;
 
-  while (L * L < Len) {
+  while ((size_t)(L * L) < Len) {
     L *= 2;
   }
 
@@ -2511,13 +2512,13 @@
 static void REC_STABLE_SORT(SORT_TYPE *arr, size_t L) {
   int m, h, p0, p1, rest;
 
-  for (m = 1; m < L; m += 2) {
+  for (m = 1; (size_t)m < L; m += 2) {
     if (SORT_CMP_A(arr + m - 1, arr + m) > 0) {
       GRAIL_SWAP1(arr + (m - 1), arr + m);
     }
   }
 
-  for (h = 2; h < L; h *= 2) {
+  for (h = 2; (size_t)h < L; h *= 2) {
     p0 = 0;
     p1 = L - 2 * h;

@swenson
Copy link
Owner

swenson commented Jan 8, 2019

It's definitely fine to fix these issues if they do not cause problems in C89 mode. But, unfortunately, we are stuck on C89 until Microsoft Visual C has supported newer standards for a while, which from I have heard, is not happening anytime soon.

@nwellnhof
Copy link
Contributor

Just to add a data point: We use sort.h in libxml2 specifically because it only requires C89 and we still want to support some exotic platforms without a C99 compiler.

@drankinatty
Copy link
Author

Ah, that makes perfect sense. I have to jump through hoops as well writing example for those still using the Windows 7.1 SDK. Nothing like having to define what "%zu" is or removing all loop variable declarations and moving them back to the top. Thanks for looking at it.

@swenson
Copy link
Owner

swenson commented Sep 21, 2019

I believe these issues have been addressed by now (regarding use of size_t, etc.). Let me know if they have not.

@swenson swenson closed this as completed Sep 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants