Replace zu with PRIsz and llu with PRIu64. #2512

SeanCampbell · 2017-10-31T17:11:27Z

The new macro, PRIsz, can be defined differently on systems that don't support PRIu64, zu, and llu, which will make the code easier to port.

SeanCampbell · 2017-10-31T17:13:50Z

@igorpeshansky - Can you take a look at this? This is based on the discussion in the windows-fixes branch in the Stackdriver fork.

igorpeshansky · 2017-10-31T21:43:58Z

src/daemon/common.h

@@ -384,4 +384,8 @@ void strarray_free(char **array, size_t array_len);
 int check_capability(int arg);
 #endif /* HAVE_SYS_CAPABILITY_H */

+#ifndef PRIsz
+#define PRIsz PRIu64


Is there a reason you didn't use the latest version of those changes?

I left out the Windows bits, since it doesn't currently run on Windows, so I couldn't make sure it all worked.

If you want, I can add them in, since it is just copying and pasting over.

Nah, I was proposing to define PRIsz in terms of what works on Linux for now, so you could add additional cases later.

igorpeshansky · 2017-10-31T21:46:24Z

src/utils_format_graphite.c

@@ -60,11 +60,11 @@ static int gr_format_values(char *ret, size_t ret_len, int ds_num,
  else if (rates != NULL)
    BUFFER_ADD("%f", rates[ds_num]);
  else if (ds->ds[ds_num].type == DS_TYPE_COUNTER)
-    BUFFER_ADD("%llu", vl->values[ds_num].counter);
+    BUFFER_ADD("%" PRIsz, (uint64_t)vl->values[ds_num].counter);


I think you need to use PRIu64 for uint64_t throughout.

igorpeshansky · 2017-10-31T21:46:58Z

src/utils_format_graphite.c

  else if (ds->ds[ds_num].type == DS_TYPE_DERIVE)
    BUFFER_ADD("%" PRIi64, vl->values[ds_num].derive);
  else if (ds->ds[ds_num].type == DS_TYPE_ABSOLUTE)
-    BUFFER_ADD("%" PRIu64, vl->values[ds_num].absolute);
+    BUFFER_ADD("%" PRIsz, vl->values[ds_num].absolute);


Ditto here, you need PRIu64 for uint64_t.

Thanks, I didn't quite get where to use PRIu64 before. Fixed.

igorpeshansky

LGTM

igorpeshansky · 2017-11-01T03:12:20Z

src/daemon/common.h

@@ -384,4 +384,8 @@ void strarray_free(char **array, size_t array_len);
 int check_capability(int arg);
 #endif /* HAVE_SYS_CAPABILITY_H */

+#ifndef PRIsz
+#define PRIsz PRIu64


Nah, I was proposing to define PRIsz in terms of what works on Linux for now, so you could add additional cases later.

igorpeshansky · 2017-11-01T17:41:05Z

@octo @tokkee This should now be ready for review...

igorpeshansky · 2017-11-03T16:42:13Z

src/postgresql.c

@@ -742,7 +742,7 @@ static char *values_to_sqlarray(const data_set_t *ds, const value_list_t *vl,

      status = snprintf(str_ptr, str_len, ",%lf", rates[i]);
    } else if (ds->ds[i].type == DS_TYPE_COUNTER)
-      status = snprintf(str_ptr, str_len, ",%llu", vl->values[i].counter);
+      status = snprintf(str_ptr, str_len, ",%" PRIu64, vl->values[i].counter);


Do you need a cast here?

I think you're right with all of these. I was able to build and install without error, but it probably wasn't building these plugins and so didn't throw any error.

igorpeshansky · 2017-11-03T16:42:23Z

src/rrdcached.c

@@ -88,7 +88,7 @@ static int value_list_to_string(char *buffer, int buffer_len,
      return -1;

    if (ds->ds[i].type == DS_TYPE_COUNTER) {
-      status = snprintf(buffer + offset, buffer_len - offset, ":%llu",
+      status = snprintf(buffer + offset, buffer_len - offset, ":%" PRIu64,
                        vl->values[i].counter);


Do you need a cast here?

igorpeshansky · 2017-11-03T16:42:40Z

src/rrdtool.c

@@ -226,7 +226,7 @@ static int value_list_to_string(char *buffer, int buffer_len,
                      vl->values[0].gauge);
    break;
  case DS_TYPE_COUNTER:
-    status = snprintf(buffer, buffer_len, "%u:%llu", (unsigned)tt,
+    status = snprintf(buffer, buffer_len, "%u:%" PRIu64, (unsigned)tt,


Do you need a different cast here?

igorpeshansky · 2017-11-03T16:42:49Z

src/snmp.c

@@ -1093,7 +1093,7 @@ static int csnmp_instance_list_add(csnmp_list_instances_t **head,
    value_t val = csnmp_value_list_to_value(
        vb, DS_TYPE_COUNTER,
        /* scale = */ 1.0, /* shift = */ 0.0, hd->name, dd->name);
-    snprintf(il->instance, sizeof(il->instance), "%llu", val.counter);
+    snprintf(il->instance, sizeof(il->instance), "%" PRIu64, val.counter);


Do you need a cast here?

igorpeshansky · 2017-11-03T16:43:06Z

src/utils_format_kairosdb.c

@@ -154,7 +154,7 @@ static int values_to_kairosdb(char *buffer, size_t buffer_size, /* {{{ */
    BUFFER_ADD("[[");
    BUFFER_ADD("%" PRIu64, CDTIME_T_TO_MS(vl->time));
    BUFFER_ADD(",");
-    BUFFER_ADD("%llu", vl->values[ds_idx].counter);
+    BUFFER_ADD("%" PRIu64, vl->values[ds_idx].counter);


Do you need a cast here?

igorpeshansky · 2017-11-03T16:47:09Z

src/email.c

  sstrncpy(addr.sun_path, path, (size_t)(UNIX_PATH_MAX - 1));

  errno = 0;
  if (bind(connector_socket, (struct sockaddr *)&addr,
-           offsetof(struct sockaddr_un, sun_path) + strlen(addr.sun_path)) == -1) {
+           offsetof(struct sockaddr_un, sun_path) + strlen(addr.sun_path)) ==
+      -1) {


I'd undo this change. But up to @octo.

igorpeshansky · 2017-11-03T16:51:05Z

src/libcollectdclient/network_parse_test.c

-          stderr,
-          "lcc_network_parse(raw_packet_data[%zu]): decoding string failed\n",
-          i);
+      fprintf(stderr, "lcc_network_parse(raw_packet_data[%" PRIsz


How about:

fprintf(stderr, "lcc_network_parse(raw_packet_data[%" PRIsz "]):" " decoding string failed\n", i);

?

igorpeshansky · 2017-11-03T16:53:19Z

src/nfs.c

@@ -488,7 +488,7 @@ static int nfs_submit_nfs4_client(const char *instance, char **fields,
    if (!suppress_warning) {
      WARNING("nfs plugin: Unexpected number of "
              "fields for NFSv4 %s "
-              "statistics: %zu. ",
+              "statistics: %" PRIsz ". ",


Join with the previous line?

igorpeshansky · 2017-11-03T16:54:47Z

src/oracle.c

@@ -532,7 +532,8 @@ static int o_read_database_query(o_database_t *db, /* {{{ */
    memcpy(column_names[i], column_name, column_name_length);
    column_names[i][column_name_length] = 0;

-    DEBUG("oracle plugin: o_read_database_query: column_names[%zu] = %s; "
+    DEBUG("oracle plugin: o_read_database_query: column_names[%" PRIsz


How about:

DEBUG("oracle plugin: o_read_database_query: column_names[%" PRIsz "] = %s;" " column_name_length = %" PRIu32 ";", i, column_names[i], (uint32_t)column_name_length);

?

igorpeshansky · 2017-11-03T16:55:49Z

src/utils_lua.c

-            "got %zu",
+    WARNING("ltoc_values: invalid size for datasource \"%s\": expected %" PRIsz
+            ", "
+            "got %" PRIsz,


Join with the previous line?

igorpeshansky · 2017-11-03T16:59:38Z

@SeanCampbell, you might also want to update the PR description (e.g., to Replace "llu" with PRIu64 and "zu" with PRIsz in format strings).
@octo, @tokkee, once this is approved, it would probably make sense to either squash-and-merge or rewrite the history on the PR branch before merging.

igorpeshansky

LGTM

SeanCampbell · 2017-11-13T14:13:46Z

Friendly ping :)

Should I deal with the conflicting files, or is that dealt with as the PR is merged?

SeanCampbell · 2017-11-22T12:44:20Z

@octo @tokkee Friendly ping on this :) Is there anything I should clean up before this is reviewed or merged?

tokkee

LGTM, thanks! 👍

However, you'll have to resolve the merge conflicts before we can merge this PR. While doing that, also consider rewriting the branch for a cleaner history.

tokkee · 2017-11-23T15:35:57Z

src/email.c

-  struct sockaddr_un addr = {
-    .sun_family = AF_UNIX
-  };
+  struct sockaddr_un addr = {.sun_family = AF_UNIX};


Yeah, this is clang-format, so that's fine.

igorpeshansky · 2017-11-23T16:04:02Z

@tokkee A general comment: GitHub now has squash-and-merge. Unless we want to merge this as multiple commits for some reason, wouldn't the final squash-and-merge "just work"?

SeanCampbell · 2017-11-27T20:23:05Z

Merged the conflicts.

For rewriting the branch, do you mean just combining the commits for this PR into one? Thanks.

… it easier to make the code platform-independent.

SeanCampbell · 2017-12-06T00:16:02Z

@tokkee Cleaned up the history commit -- what do you think?

SeanCampbell · 2017-12-13T17:11:26Z

Friendly ping

tokkee · 2017-12-15T13:08:23Z

Thanks! I would have been fine with using squash-and-merge but wanted to leave it to Sean to decide how to split this into one or more commits.

igorpeshansky requested changes Oct 31, 2017

View reviewed changes

igorpeshansky approved these changes Nov 1, 2017

View reviewed changes

igorpeshansky requested changes Nov 3, 2017

View reviewed changes

SeanCampbell changed the title ~~Replace PRIu64, zu, and llu with PRIsz.~~ Replace zu with PRIsz and llu with PRIu64. Nov 3, 2017

igorpeshansky approved these changes Nov 3, 2017

View reviewed changes

tokkee requested changes Nov 23, 2017

View reviewed changes

Replace zu with PRIu64 and llu with new macro, PRIsz, which will make…

7b8851b

… it easier to make the code platform-independent.

SeanCampbell force-pushed the pri branch from b15b8ce to 7b8851b Compare December 6, 2017 00:00

Merge branch 'master' into pri

4c4988b

tokkee approved these changes Dec 15, 2017

View reviewed changes

tokkee merged commit 77ca1a4 into collectd:master Dec 15, 2017

SeanCampbell deleted the pri branch December 15, 2017 15:29

kwiatrox mentioned this pull request Feb 22, 2018

intel_pmu: core groups feature #2681

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace zu with PRIsz and llu with PRIu64. #2512

Replace zu with PRIsz and llu with PRIu64. #2512

SeanCampbell commented Oct 31, 2017

SeanCampbell commented Oct 31, 2017

igorpeshansky Oct 31, 2017

SeanCampbell Oct 31, 2017

igorpeshansky Nov 1, 2017

igorpeshansky Oct 31, 2017

SeanCampbell Oct 31, 2017

igorpeshansky Oct 31, 2017

SeanCampbell Oct 31, 2017

igorpeshansky left a comment

igorpeshansky Nov 1, 2017

igorpeshansky commented Nov 1, 2017

igorpeshansky Nov 3, 2017

SeanCampbell Nov 3, 2017

igorpeshansky Nov 3, 2017

igorpeshansky Nov 3, 2017

igorpeshansky Nov 3, 2017

igorpeshansky Nov 3, 2017

igorpeshansky Nov 3, 2017

igorpeshansky Nov 3, 2017

igorpeshansky Nov 3, 2017

igorpeshansky Nov 3, 2017

igorpeshansky Nov 3, 2017

igorpeshansky commented Nov 3, 2017

igorpeshansky left a comment

SeanCampbell commented Nov 13, 2017

SeanCampbell commented Nov 22, 2017

tokkee left a comment

tokkee Nov 23, 2017

igorpeshansky commented Nov 23, 2017

SeanCampbell commented Nov 27, 2017

SeanCampbell commented Dec 6, 2017

SeanCampbell commented Dec 13, 2017

tokkee commented Dec 15, 2017

Replace zu with PRIsz and llu with PRIu64. #2512

Replace zu with PRIsz and llu with PRIu64. #2512

Conversation

SeanCampbell commented Oct 31, 2017

SeanCampbell commented Oct 31, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igorpeshansky left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igorpeshansky commented Nov 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igorpeshansky commented Nov 3, 2017

igorpeshansky left a comment

Choose a reason for hiding this comment

SeanCampbell commented Nov 13, 2017

SeanCampbell commented Nov 22, 2017

tokkee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igorpeshansky commented Nov 23, 2017

SeanCampbell commented Nov 27, 2017

SeanCampbell commented Dec 6, 2017

SeanCampbell commented Dec 13, 2017

tokkee commented Dec 15, 2017