Re: [PATCH 06/11] Remove always true nonnull check on "this" pointer.

[Corinna Vinschen <corinna-cygwin at cygwin dot com>]

Hi Peter,

On Mar 19 13:45, Peter Foley wrote:
> G++ 6.0 can assert that the this pointer is non-null for member functions.

Maybe, but if it compains, it's bound to find false positives...

> @@ -502,7 +502,7 @@ fhandler_dev_dsp::Audio_out::buf_info (audio_buf_info *p,
>  				       int rate, int bits, int channels)
>  {
>    p->fragstotal = MAX_BLOCKS;
> -  if (this && dev_)
> +  if (dev_)
>      {
>        /* If the device is running we use the internal values,
>  	 possibly set from the wave file. */

This is wrong.  fhandler_dev_dsp::Audio_out::buf_info is called from
fhandler_dev_dsp::_ioctl.  An application can call ioctl without
an open audio channel.  If so, audio_out_ may be NULL and the
Audio_out::buf_info method is called with a NULL this pointer..

> @@ -959,7 +959,7 @@ fhandler_dev_dsp::Audio_in::buf_info (audio_buf_info *p,
>  {
>    p->fragstotal = MAX_BLOCKS;
>    p->fragsize = blockSize (rate, bits, channels);
> -  if (this && dev_)
> +  if (dev_)

Ditto.

>      {
>        p->fragments = Qisr2app_->query ();
>        if (pHdr_ != NULL)
> diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
> index 20391bf..df09d70 100644
> --- a/winsup/cygwin/path.cc
> +++ b/winsup/cygwin/path.cc
> @@ -3937,7 +3937,7 @@ fcwd_access_t::Free (PVOID heap)
>  {
>    /* Decrement the reference count.  If it's down to 0, free
>       structure from heap. */
> -  if (this && InterlockedDecrement (&ReferenceCount ()) == 0)
> +  if (InterlockedDecrement (&ReferenceCount ()) == 0)

Very unlikely, but *fast_cwd_ptr *might be NULL.  Better save than sorry.

>      {
>        /* In contrast to pre-Vista, the handle on init is always a
>  	 fresh one and not the handle inherited from the parent
> diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
> index be32cfd..409a0b7 100644
> --- a/winsup/cygwin/pinfo.cc
> +++ b/winsup/cygwin/pinfo.cc
> @@ -514,7 +514,7 @@ _pinfo::set_ctty (fhandler_termios *fh, int flags)
>  bool __reg1
>  _pinfo::exists ()
>  {
> -  return this && process_state && !(process_state & (PID_EXITED | PID_REAPED | PID_EXECED));
> +  return process_state && !(process_state & (PID_EXITED | PID_REAPED | PID_EXECED));

Assuming a pinfo constructor called like this:

  pinfo p (non_existent_pid);

then p->_procinfo is NULL.  Calling p->exists then calls _pinfo::exists
with a NULL this pointer.  Analog for the other _pinfo methods.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

Attachment: signature.asc
Description: PGP signature