From: Peilin Ye <yepeilin.cs@gmail.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"James E . J . Bottomley" <James.Bottomley@hansenpartnership.com>,
Helge Deller <deller@gmx.de>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
Jiri Slaby <jirislaby@kernel.org>,
linux-parisc@vger.kernel.org, dri-devel@lists.freedesktop.org,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/5] Fonts: Make font size unsigned in font_desc
Date: Wed, 28 Oct 2020 06:30:50 -0400 [thread overview]
Message-ID: <20201028103050.GA1209391@PWN> (raw)
In-Reply-To: <20201028081844.GS401619@phenom.ffwll.local>
On Wed, Oct 28, 2020 at 09:18:44AM +0100, Daniel Vetter wrote:
> On Wed, Oct 28, 2020 at 01:43:07AM -0400, Peilin Ye wrote:
> > On Tue, Oct 27, 2020 at 07:50:58PM +0100, Daniel Vetter wrote:
> > > On Tue, Oct 27, 2020 at 12:33:05PM -0400, Peilin Ye wrote:
> > > > It is improper to define `width` and `height` as signed in `struct
> > > > font_desc`. Make them unsigned. Also, change the corresponding printk()
> > > > format identifiers from `%d` to `%u`, in sti_select_fbfont().
> > > >
> > > > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
> > >
> > > I'm not entirely sure of the motivation here ... height/width should never
> > > ever be even close to the limit here. Or have you seen integer math that
> > > could potentially go wrong if we go with unsigned instead of int?
> >
> > Oh... No, I have not. I just thought we shouldn't represent a length
> > using a signed value. Also, width and height in console_font are
> > unsigned int - that shouldn't matter that much though.
>
> Oh this is actually a good reason, since that's the uapi structure. And so
> using the exact same signedness should help a bit with accidental casting
> bugs.
>
> If you mention this in the commit message I think this is good to go.
Ah, I see, v2 on the way. Please ignore [v2 3/5], that doesn't hunk with
this patch in effect...
One newbie question, should I mention in the commit message, if a patch
depends on another patch in the series in order to hunk properly?
Peilin
WARNING: multiple messages have this Message-ID (diff)
From: Peilin Ye <yepeilin.cs@gmail.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: linux-fbdev@vger.kernel.org, linux-parisc@vger.kernel.org,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Helge Deller <deller@gmx.de>,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
"James E . J . Bottomley" <James.Bottomley@hansenpartnership.com>,
Jiri Slaby <jirislaby@kernel.org>
Subject: Re: [PATCH 2/5] Fonts: Make font size unsigned in font_desc
Date: Wed, 28 Oct 2020 06:30:50 -0400 [thread overview]
Message-ID: <20201028103050.GA1209391@PWN> (raw)
In-Reply-To: <20201028081844.GS401619@phenom.ffwll.local>
On Wed, Oct 28, 2020 at 09:18:44AM +0100, Daniel Vetter wrote:
> On Wed, Oct 28, 2020 at 01:43:07AM -0400, Peilin Ye wrote:
> > On Tue, Oct 27, 2020 at 07:50:58PM +0100, Daniel Vetter wrote:
> > > On Tue, Oct 27, 2020 at 12:33:05PM -0400, Peilin Ye wrote:
> > > > It is improper to define `width` and `height` as signed in `struct
> > > > font_desc`. Make them unsigned. Also, change the corresponding printk()
> > > > format identifiers from `%d` to `%u`, in sti_select_fbfont().
> > > >
> > > > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
> > >
> > > I'm not entirely sure of the motivation here ... height/width should never
> > > ever be even close to the limit here. Or have you seen integer math that
> > > could potentially go wrong if we go with unsigned instead of int?
> >
> > Oh... No, I have not. I just thought we shouldn't represent a length
> > using a signed value. Also, width and height in console_font are
> > unsigned int - that shouldn't matter that much though.
>
> Oh this is actually a good reason, since that's the uapi structure. And so
> using the exact same signedness should help a bit with accidental casting
> bugs.
>
> If you mention this in the commit message I think this is good to go.
Ah, I see, v2 on the way. Please ignore [v2 3/5], that doesn't hunk with
this patch in effect...
One newbie question, should I mention in the commit message, if a patch
depends on another patch in the series in order to hunk properly?
Peilin
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-10-28 23:42 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-27 16:27 [PATCH 0/5] Preparation work for using font_desc in vc_data Peilin Ye
2020-10-27 16:27 ` Peilin Ye
2020-10-27 16:31 ` [PATCH 1/5] fbdev/atafb: Remove unused extern variables Peilin Ye
2020-10-27 16:31 ` Peilin Ye
2020-10-27 16:33 ` [PATCH 2/5] Fonts: Make font size unsigned in font_desc Peilin Ye
2020-10-27 16:33 ` Peilin Ye
2020-10-27 16:34 ` [PATCH 3/5] Fonts: Add charcount field to font_desc Peilin Ye
2020-10-27 16:34 ` Peilin Ye
2020-10-27 16:37 ` [PATCH 4/5] fbcon: Avoid hard-coding built-in font charcount Peilin Ye
2020-10-27 16:37 ` Peilin Ye
2020-10-27 16:41 ` [PATCH 5/5] parisc/sticore: " Peilin Ye
2020-10-27 16:41 ` Peilin Ye
2020-10-27 19:18 ` Daniel Vetter
2020-10-27 19:18 ` Daniel Vetter
2020-10-27 19:13 ` [PATCH 4/5] fbcon: " Daniel Vetter
2020-10-27 19:13 ` Daniel Vetter
2020-10-28 5:30 ` Peilin Ye
2020-10-28 5:30 ` Peilin Ye
2020-10-28 15:51 ` [PATCH RFC v2 4/5] fbdev: Avoid using FNTCHARCNT() and hard-coded " Peilin Ye
2020-10-28 15:51 ` Peilin Ye
2020-10-27 18:59 ` [PATCH 3/5] Fonts: Add charcount field to font_desc Daniel Vetter
2020-10-27 18:59 ` Daniel Vetter
2020-10-28 6:11 ` Peilin Ye
2020-10-28 6:11 ` Peilin Ye
2020-10-28 6:05 ` [PATCH 3/5 v2] " Peilin Ye
2020-10-28 6:05 ` Peilin Ye
2020-11-02 15:03 ` Daniel Vetter
2020-11-02 15:03 ` Daniel Vetter
2020-10-27 18:50 ` [PATCH 2/5] Fonts: Make font size unsigned in font_desc Daniel Vetter
2020-10-27 18:50 ` Daniel Vetter
2020-10-28 5:43 ` Peilin Ye
2020-10-28 5:43 ` Peilin Ye
2020-10-28 8:18 ` Daniel Vetter
2020-10-28 8:18 ` Daniel Vetter
2020-10-28 10:30 ` Peilin Ye [this message]
2020-10-28 10:30 ` Peilin Ye
2020-10-28 10:56 ` [PATCH v2 " Peilin Ye
2020-10-28 10:56 ` Peilin Ye
2020-10-28 18:40 ` Daniel Vetter
2020-10-28 18:40 ` Daniel Vetter
2020-10-27 18:44 ` [PATCH 1/5] fbdev/atafb: Remove unused extern variables Daniel Vetter
2020-10-27 18:44 ` Daniel Vetter
2020-10-28 9:59 ` Geert Uytterhoeven
2020-10-28 9:59 ` Geert Uytterhoeven
2020-10-28 19:25 ` Thomas Zimmermann
2020-10-28 19:25 ` Thomas Zimmermann
2020-10-27 16:50 ` Following up Peilin Ye
2020-10-27 16:50 ` Peilin Ye
2020-10-27 18:36 ` Daniel Vetter
2020-10-27 18:36 ` Daniel Vetter
2020-10-28 5:34 ` Peilin Ye
2020-10-28 5:34 ` Peilin Ye
2020-11-02 15:01 ` [PATCH 0/5] Preparation work for using font_desc in vc_data Daniel Vetter
2020-11-02 15:01 ` Daniel Vetter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201028103050.GA1209391@PWN \
--to=yepeilin.cs@gmail.com \
--cc=James.Bottomley@hansenpartnership.com \
--cc=b.zolnierkie@samsung.com \
--cc=daniel@ffwll.ch \
--cc=deller@gmx.de \
--cc=dri-devel@lists.freedesktop.org \
--cc=gregkh@linuxfoundation.org \
--cc=jirislaby@kernel.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-parisc@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.