From 41abcc5825d33d78bd370997ba664e3c64eb6683 Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Tue, 5 Apr 2016 10:26:06 +0200 Subject: [PATCH] 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 --- 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 603a8d772..02335eb69 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 55944b47e..9fa2c6e03 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 81f04c97a..f0423f30f 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 (""); @@ -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 (""); @@ -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 (""); diff --git a/winsup/cygwin/fhandler_termios.cc b/winsup/cygwin/fhandler_termios.cc index 7c20e787d..983e2f99e 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 e48a2cdcd..a839c0ad7 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 e6ceba8c1..d4b2afb3d 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 dcb010be4..8dfd4ab63 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 32beb3430..981004540 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 29c090ee4..e5aab8c27 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;