Fix unreachable clause where both tmp_plugin and tmp_plugin_instance are non-empty. #3350

qingling128 · 2019-11-27T21:33:18Z

ChangeLog: Fix unreachable clause where both tmp_plugin and tmp_plugin_instance are non-empty.

qingling128 · 2019-11-27T21:35:05Z

src/aggregation.c

      ssnprintf(inst->ident.plugin_instance,
                sizeof(inst->ident.plugin_instance), "%s-%s",
                tmp_plugin_instance, AGG_FUNC_PLACEHOLDER);
+    // Both tmp_plugin and tmp_plugin_instance are non-empty.
    else


This else clause is not reachable before this fix.

kwiatrox

Good fix from logical point of view.

qingling128 · 2019-12-02T20:21:31Z

@kwiatrox - Hi, any chance you know what the clang-format — unexpected status: 500 Internal Server Error failure is about? If it's intermittent, how can I trigger it again?

rpv-tomsk · 2019-12-03T06:00:30Z

I become happy when I see red sign on clang-format CI.

mrunge

Thanks for the patch here!

mrunge · 2019-12-03T11:22:27Z

@qingling128 Thank you for your patience and your patch. Unfortunately, I don't have an (easy) way to retrigger the check. The simplest way would be to push another change to your branch.

qingling128 · 2019-12-16T21:35:49Z

Just pushed some cosmetic changes to re-trigger the builds.

mrunge · 2019-12-17T06:57:19Z

@qingling128 thank you!

Debian build fails with

usr/local/Cellar/grpc/1.25.0_1/include/grpcpp/impl/codegen/method_handler_impl.h:170:3: note: in instantiation of member function 'grpc_impl::internal::ServerStreamingHandler<collectd::Collectd::Service, collectd::QueryValuesRequest, collectd::QueryValuesResponse>::RunHandler' requested here

  ServerStreamingHandler(

  ^

collectd.grpc.pb.cc:78:11: note: in instantiation of member function 'grpc_impl::internal::ServerStreamingHandler<collectd::Collectd::Service, collectd::QueryValuesRequest, collectd::QueryValuesResponse>::ServerStreamingHandler' requested here

      new ::grpc::internal::ServerStreamingHandler< Collectd::Service, ::collectd::QueryValuesRequest, ::collectd::QueryValuesResponse>(

          ^

/usr/local/Cellar/protobuf/3.11.1/include/google/protobuf/message_lite.h:401:3: note: 'ByteSize' has been explicitly marked deprecated here

  PROTOBUF_DEPRECATED_MSG("Please use ByteSizeLong() instead")

  ^

/usr/local/Cellar/protobuf/3.11.1/include/google/protobuf/port_def.inc:153:53: note: expanded from macro 'PROTOBUF_DEPRECATED_MSG'

#define PROTOBUF_DEPRECATED_MSG(msg) __attribute__((deprecated(msg)))

                                                    ^

5 errors generated.

make[2]: *** [grpc_la-collectd.grpc.pb.lo] Error 1

make[1]: *** [all] Error 2

make: *** [distcheck] Error 1

The command "make distcheck DISTCHECK_CONFIGURE_FLAGS="--disable-dependency-tracking --enable-debug"" exited with 2.

This looks totally unrelated to the change :/

To solve that, we'll need some help from someone with permissions to fix CI.

@octo @mfournier could you have a look please, or provide

qingling128 · 2020-01-03T23:09:03Z

@mrunge - If the test failures are unrelated, can we disregard those failures?

mrunge · 2020-01-04T15:25:06Z

@qingling128 I would, if I could. Unfortunately, CI failures prevent merging.

qingling128 · 2020-01-05T07:54:07Z

@mrunge Thanks for the clarification. Anything else I could do to move this forward? @octo @mfournier Would retrying the build help (assuming it's intermittent)?

mrunge · 2020-01-05T14:01:13Z

The issue is not intermittent, I already kicked CI and it failed again.
Question is, why this patch fails, and others pass.
@octo and @mfournier are the people with powers on CI, I know @faxm0dem kindly offered his help, but has no permissions yet.

qingling128 · 2020-01-07T03:28:15Z

The error seems to be:

CXX      grpc_la-collectd.grpc.pb.lo
In file included from collectd.grpc.pb.cc:6:
In file included from ./collectd.grpc.pb.h:42:
/usr/local/Cellar/grpc/1.25.0_1/include/grpcpp/impl/codegen/proto_utils.h:52:23: error: 'ByteSize' is deprecated: Please use ByteSizeLong() instead [-Werror,-Wdeprecated-declarations]
  int byte_size = msg.ByteSize();
                      ^
/usr/local/Cellar/protobuf/3.11.1/include/google/protobuf/message_lite.h:401:3: note: 'ByteSize' has been explicitly marked deprecated here
  PROTOBUF_DEPRECATED_MSG("Please use ByteSizeLong() instead")
  ^
/usr/local/Cellar/protobuf/3.11.1/include/google/protobuf/port_def.inc:153:53: note: expanded from macro 'PROTOBUF_DEPRECATED_MSG'
#define PROTOBUF_DEPRECATED_MSG(msg) __attribute__((deprecated(msg)))
                                                    ^

And the error is exclusive to the job with Xcode: xcode11.2.

I just saw this commit #3353 that disabled Xcode and it's already merged to master. Let me try rebasing.

…are non-empty.

qingling128 · 2020-01-07T03:47:45Z

@mrunge - Looks like rebasing made the checks passed. Would you mind LGTM this one and getting it merged?

mrunge · 2020-01-07T07:53:08Z

Thank you and sorry for having this loong delay.

qingling128 · 2020-01-08T04:53:37Z

@mrunge - No worries. Thank you for the help!

qingling128 commented Nov 27, 2019

View reviewed changes

qingling128 mentioned this pull request Nov 27, 2019

Remove unreachable else clause. Stackdriver/collectd#160

Closed

kwiatrox previously approved these changes Nov 29, 2019

View reviewed changes

This comment has been minimized.

Sign in to view

mrunge previously approved these changes Dec 3, 2019

View reviewed changes

mrunge added the Automerge Labels PRs to be merged by a bot once approved label Dec 3, 2019

mrunge added this to the 5.11.0 milestone Dec 3, 2019

qingling128 dismissed stale reviews from mrunge and kwiatrox via c3de53a December 16, 2019 21:35

This comment has been minimized.

Sign in to view

This was referenced Dec 17, 2019

Remove grpc from Mac builds for now #3353

Merged

Revert "Remove grpc from Mac builds for now" #3358

Closed

qingling128 added 2 commits January 6, 2020 22:33

Fix unreachable clause where both tmp_plugin and tmp_plugin_instance …

883b41f

…are non-empty.

cosmetic changes.

d931e9e

mrunge approved these changes Jan 7, 2020

View reviewed changes

mrunge merged commit d56aca9 into collectd:master Jan 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix unreachable clause where both tmp_plugin and tmp_plugin_instance are non-empty. #3350

Fix unreachable clause where both tmp_plugin and tmp_plugin_instance are non-empty. #3350

qingling128 commented Nov 27, 2019 •

edited

qingling128 Nov 27, 2019

kwiatrox left a comment

qingling128 commented Dec 2, 2019

rpv-tomsk commented Dec 3, 2019

This comment has been minimized.

mrunge left a comment

mrunge commented Dec 3, 2019

qingling128 commented Dec 16, 2019

mrunge commented Dec 17, 2019

This comment has been minimized.

qingling128 commented Jan 3, 2020

mrunge commented Jan 4, 2020

qingling128 commented Jan 5, 2020

mrunge commented Jan 5, 2020

qingling128 commented Jan 7, 2020

qingling128 commented Jan 7, 2020

mrunge commented Jan 7, 2020

qingling128 commented Jan 8, 2020

Fix unreachable clause where both tmp_plugin and tmp_plugin_instance are non-empty. #3350

Fix unreachable clause where both tmp_plugin and tmp_plugin_instance are non-empty. #3350

Conversation

qingling128 commented Nov 27, 2019 • edited

qingling128 Nov 27, 2019

Choose a reason for hiding this comment

kwiatrox left a comment

Choose a reason for hiding this comment

qingling128 commented Dec 2, 2019

rpv-tomsk commented Dec 3, 2019

This comment has been minimized.

mrunge left a comment

Choose a reason for hiding this comment

mrunge commented Dec 3, 2019

qingling128 commented Dec 16, 2019

mrunge commented Dec 17, 2019

This comment has been minimized.

qingling128 commented Jan 3, 2020

mrunge commented Jan 4, 2020

qingling128 commented Jan 5, 2020

mrunge commented Jan 5, 2020

qingling128 commented Jan 7, 2020

qingling128 commented Jan 7, 2020

mrunge commented Jan 7, 2020

qingling128 commented Jan 8, 2020

qingling128 commented Nov 27, 2019 •

edited