Simplify: Only one fcore reference in tus_request()

Keeping a local varaible and the struct response member with a pointer to
the fcore was unnecessary complex and a possible cause for error.

Now that holding a pointer is conceptually equal to holding a lock,
we should really avoid the local variable.

Also fixes an (unlikely) leaked lock for TUS_FINAL and method != POST

src/tus_request.c

@@ -240,7 +240,6 @@ VCL_BOOL
 tus_request(VRT_CTX, struct VPFX(tus_server) *tussrv,
     struct tus_response *r, const char *url, const char *id)
 {
-	struct tus_file_core *fcore;
 	struct tus_file_disk *fdisk = NULL;
 	const char *p, *metadata = NULL, *concat = NULL;
 	struct concat_embryo embryo;
@@ -318,92 +317,86 @@ tus_request(VRT_CTX, struct VPFX(tus_server) *tussrv,
 	AZ(pthread_mutex_lock(&tussrv->mtx));
 
 	if (m == POST) {
-		fcore = tus_file_new(ctx, tussrv, type, url, id, metadata);
+		r->fcore = tus_file_new(ctx, tussrv, type, url, id, metadata);
 	} else {
-		fcore = tus_file_lookup(tussrv, url);
+		r->fcore = tus_file_lookup(tussrv, url);
 	}
 
-	if (fcore != NULL)
+	if (r->fcore != NULL)
 		r->schemeauth = WS_Copy(ctx->ws, tussrv->schemeauth, -1);
 
-	lock = tus_file_trylock(&fcore);
+	lock = tus_file_trylock(&r->fcore);
 	AZ(pthread_mutex_unlock(&tussrv->mtx));
 
 	if (lock == EBUSY) {
-		AZ(fcore);
+		AZ(r->fcore);
 		r->status = 423;
 		return (0);
 	}
 
-	if (fcore != NULL) {
+	if (r->fcore != NULL) {
 		assert(lock == 0);
-		fdisk = fcore->disk;
+		fdisk = r->fcore->disk;
 		AN(fdisk);
 		type = fdisk->type;
 	} else {
 		assert(lock == EINVAL);
 	}
 
-	// from here on, we hold a lock on fcore (if != NULL), which we either
+	// from here on, we hold a lock on r->fcore (if != NULL), which we either
 	// need to unlock or pass to the response, which unlocks via
 	// tus_task_free()
 
 	if (m == POST && type == TUS_FINAL) {
-		if (fcore == NULL) {
+		if (r->fcore == NULL) {
 			tus_file_final_abort(&embryo);
 		} else {
-			fcore = tus_file_final_birth(fcore, &embryo);
+			r->fcore = tus_file_final_birth(r->fcore, &embryo);
 			r->status = 201;
 		}
 	}
 
-	AZ(r->fcore);
 	if (m == DELETE) {
-		if (fcore == NULL) {
+		if (r->fcore == NULL) {
 			r->status = 404;
 			return (0);
 		}
 
 		AZ(pthread_mutex_lock(&tussrv->mtx));
-		tus_file_del(&fcore);
+		tus_file_del(&r->fcore);
 		AZ(pthread_mutex_unlock(&tussrv->mtx));
-		AZ(fcore);
+		AZ(r->fcore);
 		r->status = 204;
 	}
 
-	AZ(r->fcore);
 	if (m == GET) {
-		if (fcore == NULL) {
+		if (r->fcore == NULL) {
 			r->status = 404;
 			return (0);
 		}
-		AN(fcore);
+		AN(r->fcore);
 		AN(fdisk);
 		if (fdisk->location_length > 0) {
 			r->status = 301;	/* done file */
 		} else {
-			tus_file_unlock(&fcore);
-			AZ(fcore);
+			tus_file_unlock(&r->fcore);
+			AZ(r->fcore);
 			r->status = 400;
 		}
-		r->fcore = fcore;
 		return (0);
 	}
 
-	AZ(r->fcore);
-	r->fcore = fcore;
-
 	if (m == HEAD) {
-		r->status = fcore ? 200 : 404;
+		r->status = r->fcore ? 200 : 404;
 		return (0);
 	}
 
-	if (fcore == NULL) {
+	if (r->fcore == NULL) {
 		r->status = (m == POST) ? 409 : 404;
 		return (0);
 	}
 
-	AN(fcore);
+	AN(r->fcore);
 	AN(fdisk);
 
 	ct_ok = http_GetHdr(ctx->http_req, H_Content_Type, &p) &&
@@ -448,10 +441,11 @@ tus_request(VRT_CTX, struct VPFX(tus_server) *tussrv,
 	if (type == TUS_FINAL) {
 		if (m != POST) {
 			r->status = 403;
-			r->fcore = NULL;
+			tus_file_unlock(&r->fcore);
+			AZ(r->fcore);
 			return (0);
 		}
-		return (tus_request_complete(ctx, tussrv, fcore->ptr, fdisk));
+		return (tus_request_complete(ctx, tussrv, r->fcore->ptr, fdisk));
 	}
 
 	assert (type != TUS_FINAL);
@@ -466,7 +460,7 @@ tus_request(VRT_CTX, struct VPFX(tus_server) *tussrv,
 			r->status = 409;
 			return (0);
 		}
-		r->status = tus_body_to_file(ctx, fcore);
+		r->status = tus_body_to_file(ctx, r->fcore);
 		break;
 	}
 	case POST: {
@@ -487,7 +481,7 @@ tus_request(VRT_CTX, struct VPFX(tus_server) *tussrv,
 			break;
 		}
 
-		r->status = tus_body_to_file(ctx, fcore);
+		r->status = tus_body_to_file(ctx, r->fcore);
 		if (r->status != 204)
 			break;
 		r->status = 201;
@@ -499,18 +493,18 @@ tus_request(VRT_CTX, struct VPFX(tus_server) *tussrv,
 
 	if (r->status == 413) {
 		AZ(pthread_mutex_lock(&tussrv->mtx));
-		tus_file_del(&fcore);
+		tus_file_del(&r->fcore);
 		AZ(pthread_mutex_unlock(&tussrv->mtx));
-		r->fcore = NULL;
+		AZ(r->fcore);
 		return (0);
 	}
 	if (fdisk->upload_offset == fdisk->upload_length) {
-		tus_file_mmap(fcore);
-		AZ(pthread_cond_signal(&fcore->cond));
+		tus_file_mmap(r->fcore);
+		AZ(pthread_cond_signal(&r->fcore->cond));
 		if (type == TUS_PARTIAL)
 			return (0);
 		assert (type == TUS_SINGLE);
-		c = tus_body_single(ctx->req, fcore);
+		c = tus_body_single(ctx->req, r->fcore);
 		if (c == NULL) {
 			VRT_fail(ctx, "not enough workspace");
 			return (0);