* When canonical input processing is enabled reads should only return

anything, if a line is available. Fixes bug #1483.
* Replaced the request owner waiting semaphore by a condition variable
  and removed the now no longer needed SemaphorePool.



git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@22357 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/add-ons/kernel/drivers/tty/Jamfile

@@ -8,6 +8,5 @@ KernelAddon <driver>tty :
 	line_buffer.cpp
 	master.cpp
 	slave.cpp
-	SemaphorePool.cpp
+;
-	;
 

src/add-ons/kernel/drivers/tty/SemaphorePool.cpp

@@ -1,100 +0,0 @@
-/*
- * Copyright 2005-2007, Ingo Weinhold, bonefish@users.sf.net.
- * Distributed under the terms of the MIT License.
- */
-
-#include <new>
-
-#include <util/AutoLock.h>
-
-#include "SemaphorePool.h"
-
-
-status_t
-Semaphore::ZeroCount()
-{
-	int32 count;
-	status_t error = get_sem_count(fSemaphore, &count);
-	if (error != B_OK)
-		return error;
-
-	if (count > 0)
-		error = acquire_sem_etc(fSemaphore, count, B_RELATIVE_TIMEOUT, 0);
-	else if (count < 0)
-		error = release_sem_etc(fSemaphore, -count, 0);
-
-	return error;
-}
-
-
-// #pragma mark -
-
-SemaphorePool::SemaphorePool(int32 maxCached)
-	: fSemaphores(),
-	  fCount(0),
-	  fMaxCount(maxCached)
-{
-	if (fMaxCount < 0)
-		fMaxCount = 8;
-}
-
-
-SemaphorePool::~SemaphorePool()
-{
-	while (Semaphore *sem = fSemaphores.Head()) {
-		fSemaphores.Remove(sem);
-		delete sem;
-	}
-
-	mutex_destroy(&fLock);
-}
-
-
-status_t
-SemaphorePool::Init()
-{
-	return mutex_init(&fLock, "tty semaphore pool");
-}
-
-
-status_t
-SemaphorePool::Get(Semaphore *&semaphore)
-{
-	MutexLocker _(fLock);
-
-	// use a cached one, if available
-	if (fCount > 0) {
-		fCount--;
-		semaphore = fSemaphores.Head();
-		fSemaphores.Remove(semaphore);
-		return B_OK;
-	}
-
-	// create a new one
-	Semaphore *sem = new(nothrow) Semaphore(0, "pool semaphore");
-	if (!sem)
-		return B_NO_MEMORY;
-
-	semaphore = sem;
-	return B_OK;
-}
-
-
-void
-SemaphorePool::Put(Semaphore *semaphore)
-{
-	if (!semaphore)
-		return;
-
-	MutexLocker _(fLock);
-
-	if (fCount >= fMaxCount
-		|| semaphore->InitCheck() != B_OK
-		|| semaphore->ZeroCount() != B_OK) {
-		delete semaphore;
-		return;
-	}
-
-	fSemaphores.Add(semaphore);
-	fCount++;
-}

src/add-ons/kernel/drivers/tty/SemaphorePool.h

@@ -1,67 +0,0 @@
-/*
- * Copyright 2005, Ingo Weinhold, bonefish@users.sf.net.
- * Distributed under the terms of the MIT License.
- */
-#ifndef TTY_SEMAPHORE_POOL_H
-#define TTY_SEMAPHORE_POOL_H
-
-#include <lock.h>
-#include <util/DoublyLinkedList.h>
-
-class Semaphore : public DoublyLinkedListLinkImpl<Semaphore> {
-public:
-	Semaphore(sem_id sem)
-		: fSemaphore(sem)
-	{
-	}
-
-	Semaphore(int32 count, const char *name)
-	{
-		fSemaphore = create_sem(count, name);
-	}
-
-	~Semaphore()
-	{
-		if (fSemaphore >= 0)
-			delete_sem(fSemaphore);
-	}
-
-	status_t InitCheck() const
-	{
-		return (fSemaphore >= 0 ? B_OK : fSemaphore);
-	}
-
-	sem_id ID() const
-	{
-		return fSemaphore;
-	}
-
-	status_t ZeroCount();
-
-private:
-	sem_id	fSemaphore;
-
-public:
-	Semaphore	*fNext;
-};
-
-class SemaphorePool {
-public:
-	SemaphorePool(int32 maxCached);
-	~SemaphorePool();
-
-	status_t Init();
-
-	status_t Get(Semaphore *&semaphore);
-	void Put(Semaphore *semaphore);
-
-private:
-	typedef DoublyLinkedList<Semaphore> SemaphoreList;
-
-	struct mutex	fLock;
-	SemaphoreList	fSemaphores;
-	int32			fCount;
-	int32			fMaxCount;
-};
-
-#endif	// TTY_SEMAPHORE_POOL_H

src/add-ons/kernel/drivers/tty/driver.cpp

@@ -11,7 +11,6 @@
 
 #include <lock.h>
 
-#include "SemaphorePool.h"
 #include "tty_private.h"
 
 
@@ -34,8 +33,6 @@ static char *sDeviceNames[kNumTTYs * 2 + 1];
 struct mutex gGlobalTTYLock;
 struct mutex gTTYCookieLock;
 struct recursive_lock gTTYRequestLock;
-static char sSemaphorePoolBuffer[sizeof(SemaphorePool)];
-SemaphorePool *gSemaphorePool = 0;
 
 
 status_t
@@ -73,15 +70,6 @@ init_driver(void)
 		return error;
 	}
 
-	// create the semaphore pool
-	gSemaphorePool
-		= new(sSemaphorePoolBuffer) SemaphorePool(kMaxCachedSemaphores);
-	error = gSemaphorePool->Init();
-	if (error != B_OK) {
-		uninit_driver();
-		return error;
-	}
-
 	// create driver name array and initialize basic TTY structures
 
 	char letter = 'p';
@@ -124,11 +112,6 @@ uninit_driver(void)
 	for (int32 i = 0; i < (int32)kNumTTYs * 2; i++)
 		free(sDeviceNames[i]);
 
-	if (gSemaphorePool) {
-		gSemaphorePool->~SemaphorePool();
-		gSemaphorePool = NULL;
-	}
-
 	recursive_lock_destroy(&gTTYRequestLock);
 	mutex_destroy(&gTTYCookieLock);
 	mutex_destroy(&gGlobalTTYLock);

src/add-ons/kernel/drivers/tty/line_buffer.cpp

@@ -49,6 +49,24 @@ line_buffer_readable(struct line_buffer &buffer)
 }
 
 
+int32
+line_buffer_readable_line(struct line_buffer &buffer, char eol, char eof)
+{
+	size_t size = buffer.in;
+	if (size == 0)
+		return 0;
+
+	// find EOL or EOF char
+	for (size_t i = 0; i < size; i++) {
+		char c = buffer.buffer[(buffer.first + i) % buffer.size];
+		if (c == eol || c == '\n' || c == '\r' || c == eof)
+			return i + 1;
+	}
+
+	return 0;
+}
+
+
 int32
 line_buffer_writable(struct line_buffer &buffer)
 {

src/add-ons/kernel/drivers/tty/line_buffer.h

@@ -20,6 +20,7 @@ status_t init_line_buffer(struct line_buffer &buffer, size_t size);
 status_t uninit_line_buffer(struct line_buffer &buffer);
 status_t clear_line_buffer(struct line_buffer &buffer);
 int32 line_buffer_readable(struct line_buffer &buffer);
+int32 line_buffer_readable_line(struct line_buffer &buffer, char eol, char eof);
 int32 line_buffer_writable(struct line_buffer &buffer);
 ssize_t line_buffer_user_read(struct line_buffer &buffer, char *data,
 			size_t length, char eof = 0, bool* hitEOF = NULL);

src/add-ons/kernel/drivers/tty/tty.cpp

@@ -19,7 +19,6 @@
 
 #include <team.h>
 
-#include "SemaphorePool.h"
 #include "tty_private.h"
 
 
@@ -134,7 +133,8 @@ class ReaderLocker : public AbstractLocker {
 		status_t AcquireReader(bool dontBlock);
 
 	private:
-	status_t _CheckBackgroundRead() const;
+		size_t _CheckAvailableBytes() const;
+		status_t _CheckBackgroundRead() const;
 
 		struct tty		*fTTY;
 		RequestOwner	fRequestOwner;
@@ -295,7 +295,7 @@ RequestQueue::NotifyError(tty_cookie *cookie, status_t error)
 
 RequestOwner::RequestOwner()
 	:
-	fSemaphore(NULL),
+	fConditionVariable(NULL),
 	fCookie(NULL),
 	fError(B_OK),
 	fBytesNeeded(1)
@@ -364,20 +364,11 @@ RequestOwner::SetBytesNeeded(size_t bytesNeeded)
  *	The request lock MUST NOT be held!
  */
 status_t
-RequestOwner::Wait(bool interruptable, Semaphore *sem)
+RequestOwner::Wait(bool interruptable)
 {
-	TRACE(("%p->RequestOwner::Wait(%d, %p)\n", this, interruptable, sem));
+	TRACE(("%p->RequestOwner::Wait(%d)\n", this, interruptable));
 
-	// get a semaphore (if not supplied or invalid)
 	status_t error = B_OK;
-	bool ownsSem = false;
-	if (!sem || sem->InitCheck() != B_OK) {
-		error = gSemaphorePool->Get(sem);
-		if (error != B_OK)
-			return error;
-
-		ownsSem = true;
-	}
 
 	RecursiveLocker locker(gTTYRequestLock);
 
@@ -386,35 +377,35 @@ RequestOwner::Wait(bool interruptable, Semaphore *sem)
 		&& (!fRequests[0].WasNotified() || !fRequests[1].WasNotified())) {
 		// not yet done
 
-		// set the semaphore
+		// publish the condition variable
-		fSemaphore = sem;
+		ConditionVariable<> conditionVariable;
+		conditionVariable.Publish(this, "tty request");
+		fConditionVariable = &conditionVariable;
+
+		// add an entry to wait on
+		ConditionVariableEntry<> entry;
+		entry.Add(this);
 
 		locker.Unlock();
 
 		// wait
-		TRACE(("%p->RequestOwner::Wait(): acquiring semaphore...\n", this));
+		TRACE(("%p->RequestOwner::Wait(): waiting for condition...\n", this));
 
-		error = acquire_sem_etc(sem->ID(), 1,
+		error = entry.Wait(interruptable ? B_CAN_INTERRUPT : 0);
-			(interruptable ? B_CAN_INTERRUPT : 0), 0);
 
-		TRACE(("%p->RequestOwner::Wait(): semaphore acquired: %lx\n", this,
+		TRACE(("%p->RequestOwner::Wait(): condition occurred: %lx\n", this,
 			error));
 
-		// remove the semaphore
+		// remove the condition variable
-		locker.SetTo(gTTYRequestLock, false);
+		locker.Lock();
-		fSemaphore = NULL;
+		fConditionVariable = NULL;
+		conditionVariable.Unpublish();
 	}
 
 	// get the result
 	if (error == B_OK)
 		error = fError;
 
-	locker.Unlock();
-
-	// return the semaphore to the pool
-	if (ownsSem)
-		gSemaphorePool->Put(sem);
-
 	return error;
 }
 
@@ -439,18 +430,18 @@ RequestOwner::Notify(Request *request)
 	TRACE(("%p->RequestOwner::Notify(%p)\n", this, request));
 
 	if (fError == B_OK && !request->WasNotified()) {
-		bool releaseSem = false;
+		bool notify = false;
 
 		if (&fRequests[0] == request) {
-			releaseSem = fRequests[1].WasNotified();
+			notify = fRequests[1].WasNotified();
 		} else if (&fRequests[1] == request) {
-			releaseSem = fRequests[0].WasNotified();
+			notify = fRequests[0].WasNotified();
 		} else {
 			// spurious call
 		}
 
-		if (releaseSem && fSemaphore)
+		if (notify && fConditionVariable)
-			release_sem(fSemaphore->ID());
+			fConditionVariable->NotifyOne();
 	}
 }
 
@@ -464,8 +455,8 @@ RequestOwner::NotifyError(Request *request, status_t error)
 		fError = error;
 
 		if (!fRequests[0].WasNotified() || !fRequests[1].WasNotified()) {
-			if (fSemaphore)
+			if (fConditionVariable)
-				release_sem(fSemaphore->ID());
+				fConditionVariable->NotifyOne();
 		}
 	}
 }
@@ -667,7 +658,7 @@ ReaderLocker::AcquireReader(bool dontBlock)
 
 	// check, if we're first in queue, and if there is something to read
 	if (fRequestOwner.IsFirstInQueues()) {
-		fBytes = line_buffer_readable(fTTY->input_buffer);
+		fBytes = _CheckAvailableBytes();
 		if (fBytes > 0)
 			return B_OK;
 	}
@@ -686,12 +677,27 @@ ReaderLocker::AcquireReader(bool dontBlock)
 		status = _CheckBackgroundRead();
 
 	if (status == B_OK)
-		fBytes = line_buffer_readable(fTTY->input_buffer);
+		fBytes = _CheckAvailableBytes();
 
 	return status;
 }
 
 
+size_t
+ReaderLocker::_CheckAvailableBytes() const
+{
+	// Reading from the slave with canonical input processing enabled means
+	// that we read at max until hitting a line end or EOF.
+	if (!fTTY->is_master && (fTTY->settings->termios.c_lflag & ICANON) != 0) {
+		return line_buffer_readable_line(fTTY->input_buffer,
+			fTTY->settings->termios.c_cc[VEOL],
+			fTTY->settings->termios.c_cc[VEOF]);
+	}
+
+	return line_buffer_readable(fTTY->input_buffer);
+}
+
+
 status_t
 ReaderLocker::_CheckBackgroundRead() const
 {
@@ -1028,11 +1034,8 @@ tty_close_cookie(struct tty_cookie *cookie)
 
 			ttyLocker.Unlock();
 
-			// wait for our turn (we reuse the blocking semaphore, so Wait()
+			// wait for our turn
-			// can't fail due to not being able to get a semaphore)
+			requestOwner.Wait(false);
-			Semaphore sem(cookie->blocking_semaphore);
-			cookie->blocking_semaphore = -1;
-			requestOwner.Wait(false, &sem);
 
 			// re-lock
 			ttyLocker.SetTo(cookie->tty->lock, false);
@@ -1402,11 +1405,17 @@ tty_input_read(tty_cookie *cookie, void *buffer, size_t *_length)
 			return status;
 		}
 
+		size_t toRead = locker.AvailableBytes();
+		if (toRead == 0)
+			continue;
+		if (toRead > length)
+			toRead = length;
+
 		bool _hitEOF = false;
 		bool* hitEOF = (tty->pending_eof > 0 ? &_hitEOF : NULL);
 
 		bytesRead = line_buffer_user_read(tty->input_buffer, (char *)buffer,
-			length, tty->settings->termios.c_cc[VEOF], hitEOF);
+			toRead, tty->settings->termios.c_cc[VEOF], hitEOF);
 		if (bytesRead < B_OK) {
 			*_length = 0;
 			return bytesRead;

src/add-ons/kernel/drivers/tty/tty_private.h

@@ -5,15 +5,16 @@
 #ifndef TTY_PRIVATE_H
 #define TTY_PRIVATE_H
 
-
-#include <KernelExport.h>
-#include <Drivers.h>
-#include <lock.h>
-#include <fs/select_sync_pool.h>
-#include <util/DoublyLinkedList.h>
-
 #include <termios.h>
 
+#include <Drivers.h>
+#include <KernelExport.h>
+
+#include <condition_variable.h>
+#include <fs/select_sync_pool.h>
+#include <lock.h>
+#include <util/DoublyLinkedList.h>
+
 #include "line_buffer.h"
 
 
@@ -29,7 +30,6 @@ typedef status_t (*tty_service_func)(struct tty *tty, uint32 op);
 
 class RequestOwner;
 class Semaphore;
-class SemaphorePool;
 struct tty;
 struct tty_cookie;
 
@@ -87,7 +87,7 @@ class RequestOwner {
 		void SetBytesNeeded(size_t bytesNeeded);
 		size_t BytesNeeded() const	{ return fBytesNeeded; }
 
-		status_t Wait(bool interruptable, Semaphore *sem = NULL);
+		status_t Wait(bool interruptable);
 
 		bool IsFirstInQueues();
 
@@ -97,12 +97,12 @@ class RequestOwner {
 		status_t Error() const	{ return fError; }
 
 	private:
-		Semaphore		*fSemaphore;
+		ConditionVariable<>*	fConditionVariable;
-		tty_cookie		*fCookie;
+		tty_cookie*				fCookie;
-		status_t		fError;
+		status_t				fError;
-		RequestQueue	*fRequestQueues[2];
+		RequestQueue*			fRequestQueues[2];
-		Request			fRequests[2];
+		Request					fRequests[2];
-		size_t			fBytesNeeded;
+		size_t					fBytesNeeded;
 };
 
 
@@ -151,7 +151,6 @@ extern device_hooks gSlaveTTYHooks;
 extern struct mutex gGlobalTTYLock;
 extern struct mutex gTTYCookieLock;
 extern struct recursive_lock gTTYRequestLock;
-extern SemaphorePool *gSemaphorePool;
 
 
 // functions available for master/slave TTYs