On Tue, Sep 10, 2019 at 11:54 AM Nathan Chancellor
[off-list ref] wrote:

Hi Xin,

The 0day team has been doing clang builds for us and this warning popped
up. Let me know if you have any questions.

On Mon, Sep 09, 2019 at 06:44:47PM +0800, kbuild test robot wrote:

quoted

CC: kbuild-all@01.org
In-Reply-To: [ref]
References: [ref]
TO: Xin Long <lucien.xin@gmail.com>
CC: network dev <redacted>, linux-sctp@vger.kernel.org
CC: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>, Neil Horman <nhorman@tuxdriver.com>, davem@davemloft.net

Hi Xin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Xin-Long/sctp-update-from-rfc7829/20190909-160115
config: x86_64-rhel-7.6 (attached as .config)
compiler: clang version 10.0.0 (git://gitmirror/llvm_project 45a3fd206fb06f77a08968c99a8172cbf2ccdd0f)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <redacted>

All warnings (new ones prefixed by >>):

quoted

quoted

net/sctp/associola.c:799:24: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand]

                   if (transport->state && SCTP_UNCONFIRMED &&
                                        ^  ~~~~~~~~~~~~~~~~
   net/sctp/associola.c:799:24: note: use '&' for a bitwise operation
                   if (transport->state && SCTP_UNCONFIRMED &&
                                        ^~
                                        &
   net/sctp/associola.c:799:24: note: remove constant to silence this warning
                   if (transport->state && SCTP_UNCONFIRMED &&
                                       ~^~~~~~~~~~~~~~~~~~~
   1 warning generated.

vim +799 net/sctp/associola.c

   775
   776        /* Engage in transport control operations.
   777         * Mark the transport up or down and send a notification to the user.
   778         * Select and update the new active and retran paths.
   779         */
   780        void sctp_assoc_control_transport(struct sctp_association *asoc,
   781                                          struct sctp_transport *transport,
   782                                          enum sctp_transport_cmd command,
   783                                          sctp_sn_error_t error)
   784        {
   785                struct sctp_ulpevent *event;
   786                struct sockaddr_storage addr;
   787                int spc_state = 0;
   788                bool ulp_notify = true;
   789
   790                /* Record the transition on the transport.  */
   791                switch (command) {
   792                case SCTP_TRANSPORT_UP:
   793                        /* If we are moving from UNCONFIRMED state due
   794                         * to heartbeat success, report the SCTP_ADDR_CONFIRMED
   795                         * state to the user, otherwise report SCTP_ADDR_AVAILABLE.
   796                         */
   797                        if (transport->state == SCTP_PF && !asoc->pf_expose)
   798                                ulp_notify = false;
 > 799                        if (transport->state && SCTP_UNCONFIRMED &&

I assume this && should either be a '&' or '=='?

Right, it should have been "==". It was changed unintentionally
when I swapped the position of 'state' and 'SCTP_UNCONFIRMED'.

Thanks, will post v2 after others' review.

quoted

   800                            SCTP_HEARTBEAT_SUCCESS == error)
   801                                spc_state = SCTP_ADDR_CONFIRMED;
   802                        else
   803                                spc_state = SCTP_ADDR_AVAILABLE;
   804                        transport->state = SCTP_ACTIVE;
   805                        break;
   806
   807                case SCTP_TRANSPORT_DOWN:
   808                        /* If the transport was never confirmed, do not transition it
   809                         * to inactive state.  Also, release the cached route since
   810                         * there may be a better route next time.
   811                         */
   812                        if (transport->state != SCTP_UNCONFIRMED) {
   813                                transport->state = SCTP_INACTIVE;
   814                                spc_state = SCTP_ADDR_UNREACHABLE;
   815                        } else {
   816                                sctp_transport_dst_release(transport);
   817                                ulp_notify = false;
   818                        }
   819                        break;
   820
   821                case SCTP_TRANSPORT_PF:
   822                        transport->state = SCTP_PF;
   823                        if (!asoc->pf_expose)
   824                                ulp_notify = false;
   825                        else
   826                                spc_state = SCTP_ADDR_POTENTIALLY_FAILED;
   827                        break;
   828
   829                default:
   830                        return;
   831                }
   832
   833                /* Generate and send a SCTP_PEER_ADDR_CHANGE notification
   834                 * to the user.
   835                 */
   836                if (ulp_notify) {
   837                        memset(&addr, 0, sizeof(struct sockaddr_storage));
   838                        memcpy(&addr, &transport->ipaddr,
   839                               transport->af_specific->sockaddr_len);
   840
   841                        event = sctp_ulpevent_make_peer_addr_change(asoc, &addr,
   842                                                0, spc_state, error, GFP_ATOMIC);
   843                        if (event)
   844                                asoc->stream.si->enqueue_event(&asoc->ulpq, event);
   845                }
   846
   847                /* Select new active and retran paths. */
   848                sctp_select_active_and_retran_path(asoc);
   849        }
   850

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Cheers,
Nathan