| Message ID | 20180603135248.51677-1-agraf@suse.de |
|---|---|
| State | Accepted |
| Commit | 80483b2ab62ca7cd200db445b6920ee96d17df88 |
| Headers | show |
| Series | efi_loader: Fix warning in raw/cols query | expand |
On 06/03/2018 03:52 PM, Alexander Graf wrote: > The code to determine rows / cols on the screen could potentially run > into a case where it doesn't know how big the screen is. In that case, > assume 80x25. This patch may silence a compiler warning. But to me it is unclear on which path of the code we could end up actually using undefined values of cols or rows without the patch. If we are copying cols and rows from vidconsole, this patch does not make a difference. If we call query_console_serial() that routine either initializes rows and cols or returns -1 in which case we do not consume rows and cols. Could you, please, either clearly state that this patch only serves to silence an invalid compiler warning or identify the path to the consumption of uninitialized values. Best regards Heinrich > > Signed-off-by: Alexander Graf <agraf@suse.de> > --- > lib/efi_loader/efi_console.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c > index 0bfc24dbd9..ce66c935ec 100644 > --- a/lib/efi_loader/efi_console.c > +++ b/lib/efi_loader/efi_console.c > @@ -223,7 +223,7 @@ static int query_console_serial(int *rows, int *cols) > static void query_console_size(void) > { > const char *stdout_name = env_get("stdout"); > - int rows, cols; > + int rows = 25, cols = 80; > > if (stdout_name && !strcmp(stdout_name, "vidconsole") && > IS_ENABLED(CONFIG_DM_VIDEO)) { >
> Am 03.06.2018 um 19:37 schrieb Heinrich Schuchardt <xypron.debian@gmx.de>: > >> On 06/03/2018 03:52 PM, Alexander Graf wrote: >> The code to determine rows / cols on the screen could potentially run >> into a case where it doesn't know how big the screen is. In that case, >> assume 80x25. > > This patch may silence a compiler warning. But to me it is unclear on > which path of the code we could end up actually using undefined values > of cols or rows without the patch. > > If we are copying cols and rows from vidconsole, this patch does not > make a difference. > > If we call query_console_serial() that routine either initializes rows > and cols or returns -1 in which case we do not consume rows and cols. > > Could you, please, either clearly state that this patch only serves to > silence an invalid compiler warning or identify the path to the > consumption of uninitialized values. If the serial client on the other end replies with <esc>[t then the values of n are uninitialized. I have to admit that this patch doesn't help in that case either though. Alex
diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c index 0bfc24dbd9..ce66c935ec 100644 --- a/lib/efi_loader/efi_console.c +++ b/lib/efi_loader/efi_console.c @@ -223,7 +223,7 @@ static int query_console_serial(int *rows, int *cols) static void query_console_size(void) { const char *stdout_name = env_get("stdout"); - int rows, cols; + int rows = 25, cols = 80; if (stdout_name && !strcmp(stdout_name, "vidconsole") && IS_ENABLED(CONFIG_DM_VIDEO)) {
The code to determine rows / cols on the screen could potentially run into a case where it doesn't know how big the screen is. In that case, assume 80x25. Signed-off-by: Alexander Graf <agraf@suse.de> --- lib/efi_loader/efi_console.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)