From patchwork Sun Jan 31 12:25:55 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Willem Jan Withagen X-Patchwork-Id: 8174171 Return-Path: X-Original-To: patchwork-ceph-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 39D2FBEEE5 for ; Sun, 31 Jan 2016 12:26:30 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4DC55202EC for ; Sun, 31 Jan 2016 12:26:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D27C1202A1 for ; Sun, 31 Jan 2016 12:26:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757376AbcAaM00 (ORCPT ); Sun, 31 Jan 2016 07:26:26 -0500 Received: from smtp.digiware.nl ([31.223.170.169]:24417 "EHLO smtp.digiware.nl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757278AbcAaM0Z (ORCPT ); Sun, 31 Jan 2016 07:26:25 -0500 Received: from rack1.digiware.nl (unknown [127.0.0.1]) by smtp.digiware.nl (Postfix) with ESMTP id 623D71534E8; Sun, 31 Jan 2016 13:26:22 +0100 (CET) X-Virus-Scanned: amavisd-new at digiware.nl Received: from smtp.digiware.nl ([127.0.0.1]) by rack1.digiware.nl (rack1.digiware.nl [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id IiMwIJvYQL7q; Sun, 31 Jan 2016 13:25:54 +0100 (CET) Received: from [192.168.10.10] (asus [192.168.10.10]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.digiware.nl (Postfix) with ESMTPSA id 3702D15340A; Sun, 31 Jan 2016 13:25:54 +0100 (CET) Subject: [patch] Re: compiling stops at od compare To: Matt Benjamin References: <56ACB8FF.5000605@digiware.nl> <959618990.28815879.1454194644164.JavaMail.zimbra@redhat.com> <56AD4384.2030705@digiware.nl> Cc: ceph-devel@vger.kernel.org From: Willem Jan Withagen X-Enigmail-Draft-Status: N1110 Message-ID: <56ADFD53.4020401@digiware.nl> Date: Sun, 31 Jan 2016 13:25:55 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <56AD4384.2030705@digiware.nl> Sender: ceph-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org X-Spam-Status: No, score=-7.5 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 31-1-2016 00:13, Willem Jan Withagen wrote: > On 30-1-2016 23:57, Matt Benjamin wrote: >> Should we use std::min here (that might require a cast, iirc)? > > Well the most important issue I have: > while(len>0) > where len is of type size_t has only exactly one chance to be true, aka > len = 0. negative numbers do not exist. > > So casting it to int is a bad(tm) thing. > > But as I haven't designed the code, i can only react to the compiler > error and analyze it. And then my conclusion is that the cast can only > increase the chance on an error. And thus the compiler is correct in > triggering. Sorry, I did not answer your question. If using std::min requires a cast, then it will fall in the same pitfall as the current code does. if there is a std::min version that does not autocast/promote size_t to int it might work as well. Using MIN does "the right thing", without the cast. This does the job on Centos 7 for me.... --WjW >> ----- Original Message ----- >>> From: "Willem Jan Withagen" >>> To: ceph-devel@vger.kernel.org >>> Sent: Saturday, January 30, 2016 8:22:07 AM >>> Subject: compiling stops at od compare >>> >>> When trying to compile on CentOS 7, gcc = 4.8.3 >>> >>> os/bluestore/BlueFS.cc: In member function ‘int >>> BlueFS::_read(BlueFS::FileReader*, BlueFS::FileReaderBuffer*, uint64_t, >>> size_t, ceph::bufferlist*, char*)’: >>> os/bluestore/BlueFS.cc:731:31: warning: comparison between signed and >>> unsigned integer expressions [-Wsign-compare] >>> int r = MIN((int)len, left); >>> >>> This MIN is used to determine the amount of buffer that is still left >>> to be filed. >>> And here len and left are both size_t..., suggesting that both cannot be >>> negative. So either both need to be promoted/cast, or neither. >>> >>> The cast (int)len suggests that len could be negative. >>> The part where that could happen is at line 750: >>> >>> off += r; >>> len -= r; >>> ret += r; >>> >>> So there the loop exit needs len to be exactly equal to r. Even if the >>> loop specifies while(len>0). if len gets "negative" it grows into >>> something rather big. >>> >>> Now if len never gets negative then it also does not need to get cast to >>> int. If it does, then in the unsigned case it will always be larger than >>> left. >>> >>> So bottomline is that the cast serves no purpose? >>> Removing it fixes compilation. >>> >>> --WjW >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >> > > -- > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > --- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc index 1bad6a2..13555c9 100644 --- a/src/os/bluestore/BlueFS.cc +++ b/src/os/bluestore/BlueFS.cc @@ -728,7 +728,7 @@ int BlueFS::_read( left = buf->get_buf_remaining(off); dout(20) << __func__ << " left " << left << " len " << len << dendl; - int r = MIN((int)len, left); + int r = MIN(len, left); if (outbl) { bufferlist t; t.substr_of(buf->bl, off - buf->bl_off, r);