Re: [kbuild] net/ceph/messenger_v1.c:1099:23: warning: Boolean result is used in bitwise operation. Clarify expression with parentheses.

CEPH filesystem development
 help / color / mirror / Atom feed

* Re: [kbuild] net/ceph/messenger_v1.c:1099:23: warning: Boolean result is used in bitwise operation. Clarify expression with parentheses.
       [not found] <20210119194646.GA20820@kadam>
@ 2021-01-20 11:01 ` Ilya Dryomov
  2021-01-20 12:40   ` Dan Carpenter
  0 siblings, 1 reply; 3+ messages in thread
From: Ilya Dryomov @ 2021-01-20 11:01 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: kbuild, kbuild test robot, kbuild-all, LKML, Ceph Development

On Tue, Jan 19, 2021 at 8:46 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git  master
> head:   1e2a199f6ccdc15cf111d68d212e2fd4ce65682e
> commit: 2f713615ddd9d805b6c5e79c52e0e11af99d2bf1 libceph: move msgr1 protocol implementation to its own file
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
>
> cppcheck possible warnings: (new ones prefixed by >>, may not real problems)
>
> >> net/ceph/messenger_v1.c:1099:23: warning: Boolean result is used in bitwise operation. Clarify expression with parentheses. [clarifyCondition]
>      BUG_ON(!con->in_msg ^ skip);
>                          ^
>
> vim +1099 net/ceph/messenger_v1.c
>
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1033  static int read_partial_message(struct ceph_connection *con)
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1034  {
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1035          struct ceph_msg *m = con->in_msg;
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1036          int size;
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1037          int end;
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1038          int ret;
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1039          unsigned int front_len, middle_len, data_len;
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1040          bool do_datacrc = !ceph_test_opt(from_msgr(con->msgr), NOCRC);
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1041          bool need_sign = (con->peer_features & CEPH_FEATURE_MSG_AUTH);
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1042          u64 seq;
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1043          u32 crc;
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1044
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1045          dout("read_partial_message con %p msg %p\n", con, m);
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1046
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1047          /* header */
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1048          size = sizeof (con->in_hdr);
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1049          end = size;
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1050          ret = read_partial(con, end, size, &con->in_hdr);
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1051          if (ret <= 0)
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1052                  return ret;
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1053
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1054          crc = crc32c(0, &con->in_hdr, offsetof(struct ceph_msg_header, crc));
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1055          if (cpu_to_le32(crc) != con->in_hdr.crc) {
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1056                  pr_err("read_partial_message bad hdr crc %u != expected %u\n",
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1057                         crc, con->in_hdr.crc);
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1058                  return -EBADMSG;
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1059          }
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1060
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1061          front_len = le32_to_cpu(con->in_hdr.front_len);
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1062          if (front_len > CEPH_MSG_MAX_FRONT_LEN)
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1063                  return -EIO;
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1064          middle_len = le32_to_cpu(con->in_hdr.middle_len);
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1065          if (middle_len > CEPH_MSG_MAX_MIDDLE_LEN)
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1066                  return -EIO;
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1067          data_len = le32_to_cpu(con->in_hdr.data_len);
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1068          if (data_len > CEPH_MSG_MAX_DATA_LEN)
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1069                  return -EIO;
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1070
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1071          /* verify seq# */
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1072          seq = le64_to_cpu(con->in_hdr.seq);
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1073          if ((s64)seq - (s64)con->in_seq < 1) {
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1074                  pr_info("skipping %s%lld %s seq %lld expected %lld\n",
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1075                          ENTITY_NAME(con->peer_name),
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1076                          ceph_pr_addr(&con->peer_addr),
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1077                          seq, con->in_seq + 1);
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1078                  con->in_base_pos = -front_len - middle_len - data_len -
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1079                          sizeof_footer(con);
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1080                  con->in_tag = CEPH_MSGR_TAG_READY;
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1081                  return 1;
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1082          } else if ((s64)seq - (s64)con->in_seq > 1) {
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1083                  pr_err("read_partial_message bad seq %lld expected %lld\n",
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1084                         seq, con->in_seq + 1);
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1085                  con->error_msg = "bad message sequence # for incoming message";
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1086                  return -EBADE;
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1087          }
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1088
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1089          /* allocate message? */
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1090          if (!con->in_msg) {
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1091                  int skip = 0;
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1092
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1093                  dout("got hdr type %d front %d data %d\n", con->in_hdr.type,
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1094                       front_len, data_len);
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1095                  ret = ceph_con_in_msg_alloc(con, &con->in_hdr, &skip);
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1096                  if (ret < 0)
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1097                          return ret;
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1098
> 2f713615ddd9d805 Ilya Dryomov 2020-11-12 @1099                  BUG_ON(!con->in_msg ^ skip);
>
> ! has higher precedence than ^.  It's not clear that was intended
> necessarily.

Hi Dan,

This line and the surrounding code date back to 2013, commit
2f713615ddd9 just moved it.  It is correct (we either get a message
to work with or get instructed to skip, in the latter case con->in_msg
is expected to be NULL), so I'm inclined to leave it as is.

Thanks,

                Ilya

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [kbuild] net/ceph/messenger_v1.c:1099:23: warning: Boolean result is used in bitwise operation. Clarify expression with parentheses.
  2021-01-20 11:01 ` [kbuild] net/ceph/messenger_v1.c:1099:23: warning: Boolean result is used in bitwise operation. Clarify expression with parentheses Ilya Dryomov
@ 2021-01-20 12:40   ` Dan Carpenter
  2021-01-20 12:52     ` Ilya Dryomov
  0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2021-01-20 12:40 UTC (permalink / raw)
  To: Ilya Dryomov
  Cc: kbuild, kbuild test robot, kbuild-all, LKML, Ceph Development

On Wed, Jan 20, 2021 at 12:01:59PM +0100, Ilya Dryomov wrote:
> On Tue, Jan 19, 2021 at 8:46 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> >
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git   master
> > head:   1e2a199f6ccdc15cf111d68d212e2fd4ce65682e
> > commit: 2f713615ddd9d805b6c5e79c52e0e11af99d2bf1 libceph: move msgr1 protocol implementation to its own file
> > compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> >
> >
> > cppcheck possible warnings: (new ones prefixed by >>, may not real problems)
> >
> > >> net/ceph/messenger_v1.c:1099:23: warning: Boolean result is used in bitwise operation. Clarify expression with parentheses. [clarifyCondition]
> >      BUG_ON(!con->in_msg ^ skip);
> >                          ^
> >
> > vim +1099 net/ceph/messenger_v1.c
> >
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1033  static int read_partial_message(struct ceph_connection *con)
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1034  {
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1035          struct ceph_msg *m = con->in_msg;
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1036          int size;
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1037          int end;
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1038          int ret;
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1039          unsigned int front_len, middle_len, data_len;
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1040          bool do_datacrc = !ceph_test_opt(from_msgr(con->msgr), NOCRC);
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1041          bool need_sign = (con->peer_features & CEPH_FEATURE_MSG_AUTH);
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1042          u64 seq;
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1043          u32 crc;
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1044
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1045          dout("read_partial_message con %p msg %p\n", con, m);
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1046
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1047          /* header */
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1048          size = sizeof (con->in_hdr);
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1049          end = size;
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1050          ret = read_partial(con, end, size, &con->in_hdr);
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1051          if (ret <= 0)
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1052                  return ret;
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1053
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1054          crc = crc32c(0, &con->in_hdr, offsetof(struct ceph_msg_header, crc));
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1055          if (cpu_to_le32(crc) != con->in_hdr.crc) {
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1056                  pr_err("read_partial_message bad hdr crc %u != expected %u\n",
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1057                         crc, con->in_hdr.crc);
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1058                  return -EBADMSG;
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1059          }
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1060
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1061          front_len = le32_to_cpu(con->in_hdr.front_len);
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1062          if (front_len > CEPH_MSG_MAX_FRONT_LEN)
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1063                  return -EIO;
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1064          middle_len = le32_to_cpu(con->in_hdr.middle_len);
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1065          if (middle_len > CEPH_MSG_MAX_MIDDLE_LEN)
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1066                  return -EIO;
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1067          data_len = le32_to_cpu(con->in_hdr.data_len);
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1068          if (data_len > CEPH_MSG_MAX_DATA_LEN)
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1069                  return -EIO;
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1070
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1071          /* verify seq# */
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1072          seq = le64_to_cpu(con->in_hdr.seq);
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1073          if ((s64)seq - (s64)con->in_seq < 1) {
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1074                  pr_info("skipping %s%lld %s seq %lld expected %lld\n",
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1075                          ENTITY_NAME(con->peer_name),
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1076                          ceph_pr_addr(&con->peer_addr),
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1077                          seq, con->in_seq + 1);
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1078                  con->in_base_pos = -front_len - middle_len - data_len -
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1079                          sizeof_footer(con);
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1080                  con->in_tag = CEPH_MSGR_TAG_READY;
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1081                  return 1;
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1082          } else if ((s64)seq - (s64)con->in_seq > 1) {
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1083                  pr_err("read_partial_message bad seq %lld expected %lld\n",
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1084                         seq, con->in_seq + 1);
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1085                  con->error_msg = "bad message sequence # for incoming message";
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1086                  return -EBADE;
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1087          }
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1088
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1089          /* allocate message? */
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1090          if (!con->in_msg) {
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1091                  int skip = 0;
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1092
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1093                  dout("got hdr type %d front %d data %d\n", con->in_hdr.type,
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1094                       front_len, data_len);
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1095                  ret = ceph_con_in_msg_alloc(con, &con->in_hdr, &skip);
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1096                  if (ret < 0)
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1097                          return ret;
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1098
> > 2f713615ddd9d805 Ilya Dryomov 2020-11-12 @1099                  BUG_ON(!con->in_msg ^ skip);
> >
> > ! has higher precedence than ^.  It's not clear that was intended
> > necessarily.
> 
> Hi Dan,
> 
> This line and the surrounding code date back to 2013, commit
> 2f713615ddd9 just moved it.  It is correct (we either get a message
> to work with or get instructed to skip, in the latter case con->in_msg
> is expected to be NULL), so I'm inclined to leave it as is.

You could silence the warning and make the code look more intentional
by writing it like this:

	BUG_ON((!con->in_msg) ^ skip);

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [kbuild] net/ceph/messenger_v1.c:1099:23: warning: Boolean result is used in bitwise operation. Clarify expression with parentheses.
  2021-01-20 12:40   ` Dan Carpenter
@ 2021-01-20 12:52     ` Ilya Dryomov
  0 siblings, 0 replies; 3+ messages in thread
From: Ilya Dryomov @ 2021-01-20 12:52 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: kbuild, kbuild test robot, kbuild-all, LKML, Ceph Development

On Wed, Jan 20, 2021 at 1:43 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> On Wed, Jan 20, 2021 at 12:01:59PM +0100, Ilya Dryomov wrote:
> > On Tue, Jan 19, 2021 at 8:46 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > >
> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git   master
> > > head:   1e2a199f6ccdc15cf111d68d212e2fd4ce65682e
> > > commit: 2f713615ddd9d805b6c5e79c52e0e11af99d2bf1 libceph: move msgr1 protocol implementation to its own file
> > > compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> > >
> > > If you fix the issue, kindly add following tag as appropriate
> > > Reported-by: kernel test robot <lkp@intel.com>
> > >
> > >
> > > cppcheck possible warnings: (new ones prefixed by >>, may not real problems)
> > >
> > > >> net/ceph/messenger_v1.c:1099:23: warning: Boolean result is used in bitwise operation. Clarify expression with parentheses. [clarifyCondition]
> > >      BUG_ON(!con->in_msg ^ skip);
> > >                          ^
> > >
> > > vim +1099 net/ceph/messenger_v1.c
> > >
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1033  static int read_partial_message(struct ceph_connection *con)
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1034  {
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1035          struct ceph_msg *m = con->in_msg;
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1036          int size;
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1037          int end;
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1038          int ret;
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1039          unsigned int front_len, middle_len, data_len;
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1040          bool do_datacrc = !ceph_test_opt(from_msgr(con->msgr), NOCRC);
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1041          bool need_sign = (con->peer_features & CEPH_FEATURE_MSG_AUTH);
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1042          u64 seq;
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1043          u32 crc;
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1044
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1045          dout("read_partial_message con %p msg %p\n", con, m);
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1046
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1047          /* header */
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1048          size = sizeof (con->in_hdr);
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1049          end = size;
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1050          ret = read_partial(con, end, size, &con->in_hdr);
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1051          if (ret <= 0)
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1052                  return ret;
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1053
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1054          crc = crc32c(0, &con->in_hdr, offsetof(struct ceph_msg_header, crc));
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1055          if (cpu_to_le32(crc) != con->in_hdr.crc) {
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1056                  pr_err("read_partial_message bad hdr crc %u != expected %u\n",
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1057                         crc, con->in_hdr.crc);
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1058                  return -EBADMSG;
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1059          }
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1060
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1061          front_len = le32_to_cpu(con->in_hdr.front_len);
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1062          if (front_len > CEPH_MSG_MAX_FRONT_LEN)
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1063                  return -EIO;
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1064          middle_len = le32_to_cpu(con->in_hdr.middle_len);
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1065          if (middle_len > CEPH_MSG_MAX_MIDDLE_LEN)
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1066                  return -EIO;
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1067          data_len = le32_to_cpu(con->in_hdr.data_len);
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1068          if (data_len > CEPH_MSG_MAX_DATA_LEN)
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1069                  return -EIO;
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1070
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1071          /* verify seq# */
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1072          seq = le64_to_cpu(con->in_hdr.seq);
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1073          if ((s64)seq - (s64)con->in_seq < 1) {
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1074                  pr_info("skipping %s%lld %s seq %lld expected %lld\n",
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1075                          ENTITY_NAME(con->peer_name),
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1076                          ceph_pr_addr(&con->peer_addr),
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1077                          seq, con->in_seq + 1);
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1078                  con->in_base_pos = -front_len - middle_len - data_len -
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1079                          sizeof_footer(con);
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1080                  con->in_tag = CEPH_MSGR_TAG_READY;
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1081                  return 1;
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1082          } else if ((s64)seq - (s64)con->in_seq > 1) {
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1083                  pr_err("read_partial_message bad seq %lld expected %lld\n",
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1084                         seq, con->in_seq + 1);
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1085                  con->error_msg = "bad message sequence # for incoming message";
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1086                  return -EBADE;
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1087          }
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1088
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1089          /* allocate message? */
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1090          if (!con->in_msg) {
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1091                  int skip = 0;
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1092
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1093                  dout("got hdr type %d front %d data %d\n", con->in_hdr.type,
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1094                       front_len, data_len);
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1095                  ret = ceph_con_in_msg_alloc(con, &con->in_hdr, &skip);
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1096                  if (ret < 0)
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1097                          return ret;
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12  1098
> > > 2f713615ddd9d805 Ilya Dryomov 2020-11-12 @1099                  BUG_ON(!con->in_msg ^ skip);
> > >
> > > ! has higher precedence than ^.  It's not clear that was intended
> > > necessarily.
> >
> > Hi Dan,
> >
> > This line and the surrounding code date back to 2013, commit
> > 2f713615ddd9 just moved it.  It is correct (we either get a message
> > to work with or get instructed to skip, in the latter case con->in_msg
> > is expected to be NULL), so I'm inclined to leave it as is.
>
> You could silence the warning and make the code look more intentional
> by writing it like this:
>
>         BUG_ON((!con->in_msg) ^ skip);

OK, I'll do that.

Thanks,

                Ilya

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-01-20 14:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20210119194646.GA20820@kadam>
2021-01-20 11:01 ` [kbuild] net/ceph/messenger_v1.c:1099:23: warning: Boolean result is used in bitwise operation. Clarify expression with parentheses Ilya Dryomov
2021-01-20 12:40   ` Dan Carpenter
2021-01-20 12:52     ` Ilya Dryomov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox