From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::244; helo=mail-io0-x244.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x244.google.com (mail-io0-x244.google.com [IPv6:2607:f8b0:4001:c06::244]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 866B521106FBF for ; Mon, 18 Jun 2018 15:07:22 -0700 (PDT) Received: by mail-io0-x244.google.com with SMTP id l25-v6so18229211ioh.12 for ; Mon, 18 Jun 2018 15:07:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=rVZW/nixRHZKY3z/1xoMIKcv0DeXAV9qCROVSBTxvwY=; b=ERyqZvFwCmYpECpIVpTtsnKGbDI+FVnbs03QSwdHuiLP2QoTs23njr/ScYGujfrkp6 J7rfm9kgDQEx2QU7t32EXQLvfTxo/63Bkb718N9HqW3evbZRAfkRpGWcKkVO8qOFPgzl J6LexnoifxNx+uLAGNtoF1IuPB0/ySZVkpeWs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=rVZW/nixRHZKY3z/1xoMIKcv0DeXAV9qCROVSBTxvwY=; b=cBtk3e5/CJkiIIWlxHwGwEIvTQ055B4oX32w2pie/kxXa5j8VPeJB0EMF4equcjsKZ ERzLuZaoq7/7FP82UGKLNBl/hr20oia25yRgF9qbEknqA5IDdmqrVtfxGalO+ZWJTQfc 3ht/BY72+kQF8ctes1BmcBUECKjgpOavAg4mNK8/qqZNwFuaF+JIC6KXe8VtRVlQlskN zk/o0bprJ+ZPCyWuG2V63oq+0wMgul+hkPnQsNI3tsoKQowpxMV/sslnlZd+s5lRu37F 1LxAAb9pN2rUogq3c8x/h49jUk2OqCBFRe1nw4LTOF3pubIR8RyxA+/9O3P+D6dvOTMJ qdMw== X-Gm-Message-State: APt69E1Vcw7cpBp9VqR3InbYCjnHFe+47D+nOzWH51SQ25HeG6tUjhKb lyhoYciZk7ru4ThvtPkE79/NHXXxDAo76Hg8oTob2w== X-Google-Smtp-Source: ADUXVKLHvyNgqvusD/cNqbJYWGH9XN0B0MD38rA1NBGp6RoZTfboUWJmUe5jXPumzThbW8BteGFK8IcNTnrLZrGJLCA= X-Received: by 2002:a6b:520d:: with SMTP id g13-v6mr12210548iob.60.1529359641794; Mon, 18 Jun 2018 15:07:21 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:bbc7:0:0:0:0:0 with HTTP; Mon, 18 Jun 2018 15:07:21 -0700 (PDT) In-Reply-To: <20180618215758.vsoetpww4ax64v6g@bivouac.eciton.net> References: <20180618204918.31835-1-ard.biesheuvel@linaro.org> <20180618215758.vsoetpww4ax64v6g@bivouac.eciton.net> From: Ard Biesheuvel Date: Tue, 19 Jun 2018 00:07:21 +0200 Message-ID: To: Leif Lindholm Cc: "edk2-devel@lists.01.org" Subject: Re: [PATCH] EmbeddedPkg/GdbSerialLib: avoid left shift of negative quantity X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Jun 2018 22:07:22 -0000 Content-Type: text/plain; charset="UTF-8" On 18 June 2018 at 23:57, Leif Lindholm wrote: > On Mon, Jun 18, 2018 at 10:49:18PM +0200, Ard Biesheuvel wrote: >> Clang complains about left shifting a negative value being undefined. > > As well it should. > >> EmbeddedPkg/Library/GdbSerialLib/GdbSerialLib.c:151:30: >> error: shifting a negative signed value is undefined [-Werror,-Wshift-negative-value] >> OutputData = (UINT8)((~DLAB<<7)|((BreakSet<<6)|((Parity<<3)|((StopBits<<2)| Data)))); >> >> Redefine all bit pattern constants as unsigned to work around this. > > So, I'm totally OK with this and > Reviewed-by: Leif Lindholm Thanks. Pushed as 02ec23abebcb271a16fae94f8e3659bb9282880d > but ... > would it be worth fixing up BIT0-31 in Base.h and use those? > Going forward, I think it makes sense for the BITn constants to be typed as unsigned explicitly. I'm not sure what kind of toolchain pathologies we may hit, though, so I'd rather not depend on that. >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel >> --- >> EmbeddedPkg/Library/GdbSerialLib/GdbSerialLib.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/EmbeddedPkg/Library/GdbSerialLib/GdbSerialLib.c b/EmbeddedPkg/Library/GdbSerialLib/GdbSerialLib.c >> index 069d87ca780d..7931d1ac4e2b 100644 >> --- a/EmbeddedPkg/Library/GdbSerialLib/GdbSerialLib.c >> +++ b/EmbeddedPkg/Library/GdbSerialLib/GdbSerialLib.c >> @@ -40,11 +40,11 @@ >> //--------------------------------------------- >> // UART Register Bit Defines >> //--------------------------------------------- >> -#define LSR_TXRDY 0x20 >> -#define LSR_RXDA 0x01 >> -#define DLAB 0x01 >> -#define ENABLE_FIFO 0x01 >> -#define CLEAR_FIFOS 0x06 >> +#define LSR_TXRDY 0x20U >> +#define LSR_RXDA 0x01U >> +#define DLAB 0x01U >> +#define ENABLE_FIFO 0x01U >> +#define CLEAR_FIFOS 0x06U >> >> >> >> -- >> 2.17.1 >>