[Stgt-devel] [Patch] Small problem in the logging code

[FUJITA Tomonori]

On Wed, 30 Jul 2008 14:31:28 +0200
Tomas Henzl <thenzl at redhat.com> wrote:

> Hi,
> 
> I have found what I think is a small problem in the log.c. I think that
> it is caused by the fact that we are using the same sembuf struct 
> in the semop function for acquiring and releasing the semaphore and this 
> structure is shared by all threads. 
> 
> I believe that this can lead to either stopping the mechanism 
> (semop fails every time) or segfault when two threads are accessing 
> the log_enqueue at the same time.
> 
> The attached patch should solve this.
> 
> Tomas
> 
> Signed-off-by: Tomas Henzl <thenzl at redhat.com>

During the mailing list breakdown, we agreed that just using struct
sembuf on the stack is simple and fine. So the following patch was
applied.

Thanks,


=
diff --git a/usr/log.c b/usr/log.c
index 4b71216..076c770 100644
--- a/usr/log.c
+++ b/usr/log.c
@@ -108,9 +108,6 @@ static int logarea_init (int size)
 		return 1;
 	}
 
-	la->ops[0].sem_num = 0;
-	la->ops[0].sem_flg = 0;
-
 	return 0;
 }
 
@@ -237,21 +234,24 @@ static void log_syslog (void * buff)
 static void dolog(int prio, const char *fmt, va_list ap)
 {
 	struct timespec ts;
+	struct sembuf ops;
 
 	if (la) {
 		ts.tv_sec = 0;
 		ts.tv_nsec = 10000;
 
-		la->ops[0].sem_op = -1;
-		if (semtimedop(la->semid, la->ops, 1, &ts) < 0) {
+		ops.sem_num = 0;
+		ops.sem_flg = 0;
+		ops.sem_op = -1;
+		if (semtimedop(la->semid, &ops, 1, &ts) < 0) {
 			syslog(LOG_ERR, "semop up failed");
 			return;
 		}
 
 		log_enqueue(prio, fmt, ap);
 
-		la->ops[0].sem_op = 1;
-		if (semop(la->semid, la->ops, 1) < 0) {
+		ops.sem_op = 1;
+		if (semop(la->semid, &ops, 1) < 0) {
 			syslog(LOG_ERR, "semop down failed");
 			return;
 		}
@@ -291,15 +291,21 @@ void log_debug(const char *fmt, ...)
 
 static void log_flush(void)
 {
+	struct sembuf ops;
+
 	while (!la->empty) {
-		la->ops[0].sem_op = -1;
-		if (semop(la->semid, la->ops, 1) < 0) {
+		ops.sem_num = 0;
+		ops.sem_flg = 0;
+		ops.sem_op = -1;
+		if (semop(la->semid, &ops, 1) < 0) {
 			syslog(LOG_ERR, "semop up failed");
 			exit(1);
 		}
+
 		log_dequeue(la->buff);
-		la->ops[0].sem_op = 1;
-		if (semop(la->semid, la->ops, 1) < 0) {
+
+		ops.sem_op = 1;
+		if (semop(la->semid, &ops, 1) < 0) {
 			syslog(LOG_ERR, "semop down failed");
 			exit(1);
 		}
diff --git a/usr/log.h b/usr/log.h
index 6993235..b84f6d6 100644
--- a/usr/log.h
+++ b/usr/log.h
@@ -55,7 +55,6 @@ struct logarea {
 	void *start;
 	void *end;
 	char *buff;
-	struct sembuf ops[1];
 	int semid;
 	union semun semarg;
 };
-- 
1.5.5.GIT

--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html