cchar_t can contain not only attr and chars fields, but also ext_color.
Initialize the whole structure to zero instead of enumerating fields.
Spotted by Coverity: CID 1399711
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
ui/curses.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/ui/curses.c b/ui/curses.c
index d29098db9f..e99fbe3e24 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -75,9 +75,8 @@ static void curses_update(DisplayChangeListener *dcl,
if (vga_to_curses[ch].chars[0]) {
curses_line[x] = vga_to_curses[ch];
} else {
+ curses_line[x] = (cchar_t) {};
curses_line[x].chars[0] = ch;
- curses_line[x].chars[1] = 0;
- curses_line[x].attr = 0;
}
curses_line[x].attr |= at;
}
--
2.20.1
On Fri, 15 Mar 2019 at 08:37, Samuel Thibault
<samuel.thibault@ens-lyon.org> wrote:
>
> cchar_t can contain not only attr and chars fields, but also ext_color.
> Initialize the whole structure to zero instead of enumerating fields.
>
> Spotted by Coverity: CID 1399711
>
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> ui/curses.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/ui/curses.c b/ui/curses.c
> index d29098db9f..e99fbe3e24 100644
> --- a/ui/curses.c
> +++ b/ui/curses.c
> @@ -75,9 +75,8 @@ static void curses_update(DisplayChangeListener *dcl,
> if (vga_to_curses[ch].chars[0]) {
> curses_line[x] = vga_to_curses[ch];
> } else {
> + curses_line[x] = (cchar_t) {};
> curses_line[x].chars[0] = ch;
> - curses_line[x].chars[1] = 0;
> - curses_line[x].attr = 0;
> }
> curses_line[x].attr |= at;
Does this really need the cast ? {} is supposed to be a
universal initializer.
thanks
-- PMM
Peter Maydell, le ven. 15 mars 2019 10:06:48 +0000, a ecrit:
> > + curses_line[x] = (cchar_t) {};
> > curses_line[x].chars[0] = ch;
> > - curses_line[x].chars[1] = 0;
> > - curses_line[x].attr = 0;
> > }
> > curses_line[x].attr |= at;
>
> Does this really need the cast ?
Yes, otherwise it is refused by the compiler.
> {} is supposed to be a universal initializer.
When used as initialized (cchar_t foo = {}), yes. But when used in
assignations, no, one has to cast it.
Samuel
On Fri, 15 Mar 2019 at 10:52, Samuel Thibault <samuel.thibault@gnu.org> wrote:
>
> Peter Maydell, le ven. 15 mars 2019 10:06:48 +0000, a ecrit:
> > > + curses_line[x] = (cchar_t) {};
> > > curses_line[x].chars[0] = ch;
> > > - curses_line[x].chars[1] = 0;
> > > - curses_line[x].attr = 0;
> > > }
> > > curses_line[x].attr |= at;
> >
> > Does this really need the cast ?
>
> Yes, otherwise it is refused by the compiler.
>
> > {} is supposed to be a universal initializer.
>
> When used as initialized (cchar_t foo = {}), yes. But when used in
> assignations, no, one has to cast it.
Ah -- I hadn't realized that wrinkle; thanks for the explanation.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
On 3/15/19 5:06 AM, Peter Maydell wrote:
> On Fri, 15 Mar 2019 at 08:37, Samuel Thibault
> <samuel.thibault@ens-lyon.org> wrote:
>>
>> cchar_t can contain not only attr and chars fields, but also ext_color.
>> Initialize the whole structure to zero instead of enumerating fields.
>>
>> Spotted by Coverity: CID 1399711
>>
>> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
>> ---
>> ui/curses.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/ui/curses.c b/ui/curses.c
>> index d29098db9f..e99fbe3e24 100644
>> --- a/ui/curses.c
>> +++ b/ui/curses.c
>> @@ -75,9 +75,8 @@ static void curses_update(DisplayChangeListener *dcl,
>> if (vga_to_curses[ch].chars[0]) {
>> curses_line[x] = vga_to_curses[ch];
>> } else {
>> + curses_line[x] = (cchar_t) {};
>> curses_line[x].chars[0] = ch;
>> - curses_line[x].chars[1] = 0;
>> - curses_line[x].attr = 0;
>> }
>> curses_line[x].attr |= at;
>
> Does this really need the cast ? {} is supposed to be a
> universal initializer.
Or is it worth using:
curses_line[x] = (cchar_t) {
.chars[0] = ch,
};
so that all other fields not mentioned are zero-initialized, without
relying on the {} extension?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Eric Blake, le ven. 15 mars 2019 08:02:29 -0500, a ecrit:
> On 3/15/19 5:06 AM, Peter Maydell wrote:
> > On Fri, 15 Mar 2019 at 08:37, Samuel Thibault
> > <samuel.thibault@ens-lyon.org> wrote:
> >>
> >> cchar_t can contain not only attr and chars fields, but also ext_color.
> >> Initialize the whole structure to zero instead of enumerating fields.
> >>
> >> Spotted by Coverity: CID 1399711
> >>
> >> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> >> ---
> >> ui/curses.c | 3 +--
> >> 1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/ui/curses.c b/ui/curses.c
> >> index d29098db9f..e99fbe3e24 100644
> >> --- a/ui/curses.c
> >> +++ b/ui/curses.c
> >> @@ -75,9 +75,8 @@ static void curses_update(DisplayChangeListener *dcl,
> >> if (vga_to_curses[ch].chars[0]) {
> >> curses_line[x] = vga_to_curses[ch];
> >> } else {
> >> + curses_line[x] = (cchar_t) {};
> >> curses_line[x].chars[0] = ch;
> >> - curses_line[x].chars[1] = 0;
> >> - curses_line[x].attr = 0;
> >> }
> >> curses_line[x].attr |= at;
> >
> > Does this really need the cast ? {} is supposed to be a
> > universal initializer.
>
> Or is it worth using:
>
> curses_line[x] = (cchar_t) {
> .chars[0] = ch,
> };
>
> so that all other fields not mentioned are zero-initialized, without
> relying on the {} extension?
Right, it's no less readable :) I have sent an updated patch.
Samuel
© 2016 - 2026 Red Hat, Inc.