[devel] [PATCH for apt v2 21/21] Fix invalid check of Queue against zero

[Andrey Savchenko]

On Thu, 12 Dec 2019 12:57:30 +0300 Aleksei Nikiforov wrote:
> Queue must not be zero in this function, otherwise it'd crash in this function
> anyway, since it's used like it's never zero later.
> Found via clang-static-analyzer:
> Logic error: Called C++ object pointer is null:
> Called C++ object pointer is null
> ---
>  apt/apt-pkg/acquire-method.cc | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/apt/apt-pkg/acquire-method.cc b/apt/apt-pkg/acquire-method.cc
> index 9a3ef1d..3b5c580 100644
> --- a/apt/apt-pkg/acquire-method.cc
> +++ b/apt/apt-pkg/acquire-method.cc
> @@ -555,9 +555,7 @@ void pkgAcqMethod::Warning(const char *Format,...)
>     to keep the pipeline synchronized. */
>  void pkgAcqMethod::Redirect(const string &NewURI)
>  {
> -   string CurrentURI = "<UNKNOWN>";
> -   if (Queue != 0)
> -      CurrentURI = Queue->Uri;
> +   string CurrentURI = Queue->Uri;

If (Queue == NULL) this code will result in the NULL pointer
dereference. It does not follow from the code that Queue is never
NULL.

So this may be a problem in the static analyzer. Even if it is not,
such safeguard elimination is not safe for future code
modifications.

BTW GCC is good at DCE (dead code elimination), so if this check is
really useless, it will be omitted from the binary code.

>     ostringstream s;
>     s << "103 Redirect\nURI: " << CurrentURI << "\nNew-URI: " << NewURI 


Best regards,
Andrew Savchenko
----------- следующая часть -----------
Было удалено вложение не в текстовом формате...
Имя     : отсутствует
Тип     : application/pgp-signature
Размер  : 833 байтов
Описание: отсутствует
Url     : <http://lists.altlinux.org/pipermail/devel/attachments/20191212/60649d2f/attachment.bin>