This is the mail archive of the
cygwin-cvs@cygwin.com
mailing list for the Cygwin project.
[newlib-cygwin] Revert "Refactor to avoid nonnull checks on "this" pointer."
- From: Corinna Vinschen <corinna at sourceware dot org>
- To: cygwin-cvs at sourceware dot org
- Date: 5 Apr 2016 08:32:11 -0000
- Subject: [newlib-cygwin] Revert "Refactor to avoid nonnull checks on "this" pointer."
https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=41abcc5825d33d78bd370997ba664e3c64eb6683
commit 41abcc5825d33d78bd370997ba664e3c64eb6683
Author: Corinna Vinschen <corinna@vinschen.de>
Date: Tue Apr 5 10:26:06 2016 +0200
Revert "Refactor to avoid nonnull checks on "this" pointer."
This reverts commit 0008bdea02b690ab19ffe997499cb9a96ee5a66d.
This patch introduced a regression. Calling FOO=$(...) in zsh hangs
indefinitely and has to be killed forcefully.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
Diff:
---
winsup/cygwin/external.cc | 2 +-
winsup/cygwin/fhandler_dsp.cc | 55 +++++++++++----------------------------
winsup/cygwin/fhandler_process.cc | 11 +++-----
winsup/cygwin/fhandler_termios.cc | 2 +-
winsup/cygwin/path.cc | 5 ++--
winsup/cygwin/pinfo.cc | 16 ++++++------
winsup/cygwin/signal.cc | 12 +++------
winsup/cygwin/sigproc.cc | 5 ++--
winsup/cygwin/times.cc | 5 ++--
9 files changed, 38 insertions(+), 75 deletions(-)
diff --git a/winsup/cygwin/external.cc b/winsup/cygwin/external.cc
index 603a8d7..02335eb 100644
--- a/winsup/cygwin/external.cc
+++ b/winsup/cygwin/external.cc
@@ -342,7 +342,7 @@ cygwin_internal (cygwin_getinfo_types t, ...)
size_t n;
pid_t pid = va_arg (arg, pid_t);
pinfo p (pid);
- res = p ? (uintptr_t) p->cmdline (n) : 0;
+ res = (uintptr_t) p->cmdline (n);
}
break;
case CW_CHECK_NTSEC:
diff --git a/winsup/cygwin/fhandler_dsp.cc b/winsup/cygwin/fhandler_dsp.cc
index 55944b4..9fa2c6e 100644
--- a/winsup/cygwin/fhandler_dsp.cc
+++ b/winsup/cygwin/fhandler_dsp.cc
@@ -65,7 +65,7 @@ class fhandler_dev_dsp::Audio
void convert_S16LE_S16BE (unsigned char *buffer, int size_bytes);
void fillFormat (WAVEFORMATEX * format,
int rate, int bits, int channels);
- static unsigned blockSize (int rate, int bits, int channels);
+ unsigned blockSize (int rate, int bits, int channels);
void (fhandler_dev_dsp::Audio::*convert_)
(unsigned char *buffer, int size_bytes);
@@ -117,7 +117,6 @@ class fhandler_dev_dsp::Audio_out: public Audio
void stop (bool immediately = false);
int write (const char *pSampleData, int nBytes);
void buf_info (audio_buf_info *p, int rate, int bits, int channels);
- static void default_buf_info (audio_buf_info *p, int rate, int bits, int channels);
void callback_sampledone (WAVEHDR *pHdr);
bool parsewav (const char *&pData, int &nBytes,
int rate, int bits, int channels);
@@ -152,7 +151,6 @@ public:
void stop ();
bool read (char *pSampleData, int &nBytes);
void buf_info (audio_buf_info *p, int rate, int bits, int channels);
- static void default_buf_info (audio_buf_info *p, int rate, int bits, int channels);
void callback_blockfull (WAVEHDR *pHdr);
private:
@@ -503,11 +501,11 @@ void
fhandler_dev_dsp::Audio_out::buf_info (audio_buf_info *p,
int rate, int bits, int channels)
{
- if (dev_)
+ p->fragstotal = MAX_BLOCKS;
+ if (this && dev_)
{
/* If the device is running we use the internal values,
possibly set from the wave file. */
- p->fragstotal = MAX_BLOCKS;
p->fragsize = blockSize (freq_, bits_, channels_);
p->fragments = Qisr2app_->query ();
if (pHdr_ != NULL)
@@ -518,17 +516,10 @@ fhandler_dev_dsp::Audio_out::buf_info (audio_buf_info *p,
}
else
{
- default_buf_info(p, rate, bits, channels);
- }
-}
-
-void fhandler_dev_dsp::Audio_out::default_buf_info (audio_buf_info *p,
- int rate, int bits, int channels)
-{
- p->fragstotal = MAX_BLOCKS;
p->fragsize = blockSize (rate, bits, channels);
p->fragments = MAX_BLOCKS;
p->bytes = p->fragsize * p->fragments;
+ }
}
/* This is called on an interupt so use locking.. Note Qisr2app_
@@ -962,23 +953,14 @@ fhandler_dev_dsp::Audio_in::waitfordata ()
return true;
}
-void fhandler_dev_dsp::Audio_in::default_buf_info (audio_buf_info *p,
- int rate, int bits, int channels)
-{
- p->fragstotal = MAX_BLOCKS;
- p->fragsize = blockSize (rate, bits, channels);
- p->fragments = 0;
- p->bytes = 0;
-}
-
void
fhandler_dev_dsp::Audio_in::buf_info (audio_buf_info *p,
int rate, int bits, int channels)
{
- if (dev_)
+ p->fragstotal = MAX_BLOCKS;
+ p->fragsize = blockSize (rate, bits, channels);
+ if (this && dev_)
{
- p->fragstotal = MAX_BLOCKS;
- p->fragsize = blockSize (rate, bits, channels);
p->fragments = Qisr2app_->query ();
if (pHdr_ != NULL)
p->bytes = pHdr_->dwBytesRecorded - bufferIndex_
@@ -988,7 +970,8 @@ fhandler_dev_dsp::Audio_in::buf_info (audio_buf_info *p,
}
else
{
- default_buf_info(p, rate, bits, channels);
+ p->fragments = 0;
+ p->bytes = 0;
}
}
@@ -1362,13 +1345,9 @@ fhandler_dev_dsp::_ioctl (unsigned int cmd, void *buf)
return -1;
}
audio_buf_info *p = (audio_buf_info *) buf;
- if (audio_out_) {
- audio_out_->buf_info (p, audiofreq_, audiobits_, audiochannels_);
- } else {
- Audio_out::default_buf_info(p, audiofreq_, audiobits_, audiochannels_);
- }
- debug_printf ("buf=%p frags=%d fragsize=%d bytes=%d",
- buf, p->fragments, p->fragsize, p->bytes);
+ audio_out_->buf_info (p, audiofreq_, audiobits_, audiochannels_);
+ debug_printf ("buf=%p frags=%d fragsize=%d bytes=%d",
+ buf, p->fragments, p->fragsize, p->bytes);
return 0;
}
@@ -1380,13 +1359,9 @@ fhandler_dev_dsp::_ioctl (unsigned int cmd, void *buf)
return -1;
}
audio_buf_info *p = (audio_buf_info *) buf;
- if (audio_in_) {
- audio_in_->buf_info (p, audiofreq_, audiobits_, audiochannels_);
- } else {
- Audio_in::default_buf_info(p, audiofreq_, audiobits_, audiochannels_);
- }
- debug_printf ("buf=%p frags=%d fragsize=%d bytes=%d",
- buf, p->fragments, p->fragsize, p->bytes);
+ audio_in_->buf_info (p, audiofreq_, audiobits_, audiochannels_);
+ debug_printf ("buf=%p frags=%d fragsize=%d bytes=%d",
+ buf, p->fragments, p->fragsize, p->bytes);
return 0;
}
diff --git a/winsup/cygwin/fhandler_process.cc b/winsup/cygwin/fhandler_process.cc
index 81f04c9..f0423f3 100644
--- a/winsup/cygwin/fhandler_process.cc
+++ b/winsup/cygwin/fhandler_process.cc
@@ -371,11 +371,6 @@ format_process_fd (void *data, char *&destbuf)
case a trailing slash and more followup chars are allowed, provided the
descriptor symlink points to a directory. */
char *fdp = strchr (path, '/') + 3;
- if (!p)
- {
- set_errno (ENOENT);
- return 0;
- }
/* The "fd" directory itself? */
if (fdp[0] =='\0' || (fdp[0] == '/' && fdp[1] == '\0'))
{
@@ -484,7 +479,7 @@ format_process_root (void *data, char *&destbuf)
cfree (destbuf);
destbuf = NULL;
}
- destbuf = p ? p->root (fs) : NULL;
+ destbuf = p->root (fs);
if (!destbuf || !*destbuf)
{
destbuf = cstrdup ("<defunct>");
@@ -504,7 +499,7 @@ format_process_cwd (void *data, char *&destbuf)
cfree (destbuf);
destbuf = NULL;
}
- destbuf = p ? p->cwd (fs) : NULL;
+ destbuf = p->cwd (fs);
if (!destbuf || !*destbuf)
{
destbuf = cstrdup ("<defunct>");
@@ -524,7 +519,7 @@ format_process_cmdline (void *data, char *&destbuf)
cfree (destbuf);
destbuf = NULL;
}
- destbuf = p ? p->cmdline (fs) : NULL;
+ destbuf = p->cmdline (fs);
if (!destbuf || !*destbuf)
{
destbuf = cstrdup ("<defunct>");
diff --git a/winsup/cygwin/fhandler_termios.cc b/winsup/cygwin/fhandler_termios.cc
index 7c20e78..983e2f9 100644
--- a/winsup/cygwin/fhandler_termios.cc
+++ b/winsup/cygwin/fhandler_termios.cc
@@ -134,7 +134,7 @@ tty_min::kill_pgrp (int sig)
for (unsigned i = 0; i < pids.npids; i++)
{
_pinfo *p = pids[i];
- if (!p || !p->exists () || p->ctty != ntty || p->pgid != pgid)
+ if (!p->exists () || p->ctty != ntty || p->pgid != pgid)
continue;
if (p == myself)
killself = sig != __SIGSETPGRP && !exit_state;
diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index e48a2cd..a839c0a 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -3932,7 +3932,7 @@ fcwd_access_t::Free (PVOID heap)
{
/* Decrement the reference count. If it's down to 0, free
structure from heap. */
- if (InterlockedDecrement (&ReferenceCount ()) == 0)
+ if (this && InterlockedDecrement (&ReferenceCount ()) == 0)
{
/* In contrast to pre-Vista, the handle on init is always a
fresh one and not the handle inherited from the parent
@@ -4320,8 +4320,7 @@ cwdstuff::override_win32_cwd (bool init, ULONG old_dismount_count)
f_cwd->CopyPath (upp_cwd_str);
upp_cwd_hdl = dir;
RtlLeaveCriticalSection (peb.FastPebLock);
- if (old_cwd)
- old_cwd->Free (heap);
+ old_cwd->Free (heap);
}
else
{
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index e6ceba8..d4b2afb 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 process_state && !(process_state & (PID_EXITED | PID_REAPED | PID_EXECED));
+ return this && process_state && !(process_state & (PID_EXITED | PID_REAPED | PID_EXECED));
}
bool
@@ -685,7 +685,7 @@ _pinfo::commune_request (__uint32_t code, ...)
res.s = NULL;
res.n = 0;
- if (!pid)
+ if (!this || !pid)
{
set_errno (ESRCH);
goto err;
@@ -783,7 +783,7 @@ out:
fhandler_pipe *
_pinfo::pipe_fhandler (int64_t unique_id, size_t &n)
{
- if (!pid)
+ if (!this || !pid)
return NULL;
if (pid == myself->pid)
return NULL;
@@ -796,7 +796,7 @@ char *
_pinfo::fd (int fd, size_t &n)
{
char *s;
- if (!pid)
+ if (!this || !pid)
return NULL;
if (pid != myself->pid)
{
@@ -820,7 +820,7 @@ char *
_pinfo::fds (size_t &n)
{
char *s;
- if (!pid)
+ if (!this || !pid)
return NULL;
if (pid != myself->pid)
{
@@ -848,7 +848,7 @@ char *
_pinfo::root (size_t& n)
{
char *s;
- if (!pid)
+ if (!this || !pid)
return NULL;
if (pid != myself->pid && !ISSTATE (this, PID_NOTCYGWIN))
{
@@ -893,7 +893,7 @@ char *
_pinfo::cwd (size_t& n)
{
char *s = NULL;
- if (!pid)
+ if (!this || !pid)
return NULL;
if (ISSTATE (this, PID_NOTCYGWIN))
{
@@ -939,7 +939,7 @@ char *
_pinfo::cmdline (size_t& n)
{
char *s = NULL;
- if (!pid)
+ if (!this || !pid)
return NULL;
if (ISSTATE (this, PID_NOTCYGWIN))
{
diff --git a/winsup/cygwin/signal.cc b/winsup/cygwin/signal.cc
index dcb010b..8dfd4ab 100644
--- a/winsup/cygwin/signal.cc
+++ b/winsup/cygwin/signal.cc
@@ -263,7 +263,7 @@ _pinfo::kill (siginfo_t& si)
}
this_pid = pid;
}
- else if (si.si_signo == 0 && process_state == PID_EXITED)
+ else if (si.si_signo == 0 && this && process_state == PID_EXITED)
{
this_process_state = process_state;
this_pid = pid;
@@ -299,12 +299,8 @@ kill0 (pid_t pid, siginfo_t& si)
syscall_printf ("signal %d out of range", si.si_signo);
return -1;
}
- if (pid > 0) {
- pinfo p(pid);
- return p && p->kill(si);
- } else {
- return kill_pgrp(-pid, si);
- }
+
+ return (pid > 0) ? pinfo (pid)->kill (si) : kill_pgrp (-pid, si);
}
int
@@ -330,7 +326,7 @@ kill_pgrp (pid_t pid, siginfo_t& si)
{
_pinfo *p = pids[i];
- if (!p || !p->exists ())
+ if (!p->exists ())
continue;
/* Is it a process we want to kill? */
diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
index 32beb34..9810045 100644
--- a/winsup/cygwin/sigproc.cc
+++ b/winsup/cygwin/sigproc.cc
@@ -155,8 +155,7 @@ proc_can_be_signalled (_pinfo *p)
bool __reg1
pid_exists (pid_t pid)
{
- pinfo p(pid);
- return p && p->exists ();
+ return pinfo (pid)->exists ();
}
/* Return true if this is one of our children, false otherwise. */
@@ -1144,7 +1143,7 @@ remove_proc (int ci)
if (_my_tls._ctinfo != procs[ci].wait_thread)
procs[ci].wait_thread->terminate_thread ();
}
- else if (procs[ci] && procs[ci]->exists ())
+ else if (procs[ci]->exists ())
return true;
sigproc_printf ("removing procs[%d], pid %d, nprocs %d", ci, procs[ci]->pid,
diff --git a/winsup/cygwin/times.cc b/winsup/cygwin/times.cc
index 29c090e..e5aab8c 100644
--- a/winsup/cygwin/times.cc
+++ b/winsup/cygwin/times.cc
@@ -542,7 +542,7 @@ clock_gettime (clockid_t clk_id, struct timespec *tp)
pid = getpid ();
pinfo p (pid);
- if (!p || !p->exists ())
+ if (!p->exists ())
{
set_errno (EINVAL);
return -1;
@@ -765,8 +765,7 @@ clock_setres (clockid_t clk_id, struct timespec *tp)
extern "C" int
clock_getcpuclockid (pid_t pid, clockid_t *clk_id)
{
- pinfo p(pid);
- if (pid != 0 && (!p || !p->exists ()))
+ if (pid != 0 && !pinfo (pid)->exists ())
return (ESRCH);
*clk_id = (clockid_t) PID_TO_CLOCKID (pid);
return 0;