s3-printing: return talloced print jobs

print_job_find() currently returns print jobs to callers via a
statically allocated variable, this is particularly messy as the
device mode is talloced under the static variable.

This change adds or passes a talloc context to all callers, giving them
ownership of the returned print job.

source3/include/printing.h

@@ -195,7 +195,9 @@ bool print_notify_register_pid(int snum);
 bool print_notify_deregister_pid(int snum);
 bool print_job_exists(const char* sharename, uint32 jobid);
 char *print_job_fname(const char* sharename, uint32 jobid);
-struct spoolss_DeviceMode *print_job_devmode(const char* sharename, uint32 jobid);
+struct spoolss_DeviceMode *print_job_devmode(TALLOC_CTX *mem_ctx,
+					     const char *sharename,
+					     uint32 jobid);
 bool print_job_set_name(struct tevent_context *ev,
 			struct messaging_context *msg_ctx,
 			const char *sharename, uint32 jobid, const char *name);

source3/printing/printing.c

@@ -380,6 +380,7 @@ done:
  unpack a pjob from a tdb buffer
 ***********************************************************************/
 
+/* FIXME talloc ctx */
 static int unpack_pjob(uint8 *buf, int buflen, struct printjob *pjob)
 {
 	int	len = 0;
@@ -438,9 +439,11 @@ static int unpack_pjob(uint8 *buf, int buflen, struct printjob *pjob)
  Useful function to find a print job in the database.
 ****************************************************************************/
 
-static struct printjob *print_job_find(const char *sharename, uint32 jobid)
+static struct printjob *print_job_find(TALLOC_CTX *mem_ctx,
+				       const char *sharename,
+				       uint32 jobid)
 {
-	static struct printjob 	pjob;
+	struct printjob		*pjob;
 	uint32_t tmp;
 	TDB_DATA 		ret;
 	struct tdb_print_db 	*pdb = get_print_db_byname(sharename);
@@ -456,27 +459,30 @@ static struct printjob *print_job_find(const char *sharename, uint32 jobid)
 	release_print_db(pdb);
 
 	if (!ret.dptr) {
-		DEBUG(10,("print_job_find: failed to find jobid %u.\n", (unsigned int)jobid ));
+		DEBUG(10, ("print_job_find: failed to find jobid %u.\n",
+			   jobid));
 		return NULL;
 	}
 
-	talloc_free(pjob.devmode);
+	pjob = talloc_zero(mem_ctx, struct printjob);
-
+	if (pjob == NULL) {
-	ZERO_STRUCT( pjob );
+		goto err_out;
-
-	if ( unpack_pjob( ret.dptr, ret.dsize, &pjob ) == -1 ) {
-		DEBUG(10,("print_job_find: failed to unpack jobid %u.\n", (unsigned int)jobid ));
-		SAFE_FREE(ret.dptr);
-		return NULL;
 	}
 
-	SAFE_FREE(ret.dptr);
+	if (unpack_pjob(ret.dptr, ret.dsize, pjob) == -1) {
+		DEBUG(10, ("failed to unpack jobid %u.\n", jobid));
+		talloc_free(pjob);
+		pjob = NULL;
+		goto err_out;
+	}
 
 	DEBUG(10,("print_job_find: returning system job %d for jobid %u.\n",
-			(int)pjob.sysjob, (unsigned int)jobid ));
+		  pjob->sysjob, jobid));
-	SMB_ASSERT(pjob.jobid == jobid);
+	SMB_ASSERT(pjob->jobid == jobid);
 
-	return &pjob;
+err_out:
+	SAFE_FREE(ret.dptr);
+	return pjob;
 }
 
 /* Convert a unix jobid to a smb jobid */
@@ -847,19 +853,21 @@ static void pjob_delete(struct tevent_context *ev,
 	struct printjob *pjob;
 	uint32 job_status = 0;
 	struct tdb_print_db *pdb;
-
+	TALLOC_CTX *tmp_ctx = talloc_new(ev);
-	pdb = get_print_db_byname( sharename );
+	if (tmp_ctx == NULL) {
-
-	if (!pdb)
 		return;
+	}
 
-	pjob = print_job_find( sharename, jobid );
+	pdb = get_print_db_byname(sharename);
+	if (!pdb) {
+		goto err_out;
+	}
 
+	pjob = print_job_find(tmp_ctx, sharename, jobid);
 	if (!pjob) {
-		DEBUG(5, ("pjob_delete: we were asked to delete nonexistent job %u\n",
+		DEBUG(5, ("we were asked to delete nonexistent job %u\n",
-					(unsigned int)jobid));
+			  jobid));
-		release_print_db(pdb);
+		goto err_release;
-		return;
 	}
 
 	/* We must cycle through JOB_STATUS_DELETING and
@@ -873,8 +881,11 @@ static void pjob_delete(struct tevent_context *ev,
 
 	tdb_delete(pdb->tdb, print_key(jobid, &tmp));
 	remove_from_jobs_added(sharename, jobid);
-	release_print_db( pdb );
 	rap_jobid_delete(sharename, jobid);
+err_release:
+	release_print_db(pdb);
+err_out:
+	talloc_free(tmp_ctx);
 }
 
 /****************************************************************************
@@ -887,13 +898,18 @@ static void print_unix_job(struct tevent_context *ev,
 			   uint32 jobid)
 {
 	struct printjob pj, *old_pj;
+	TALLOC_CTX *tmp_ctx = talloc_new(ev);
+	if (tmp_ctx == NULL) {
+		return;
+	}
 
-	if (jobid == (uint32)-1)
+	if (jobid == (uint32)-1) {
 		jobid = q->sysjob + UNIX_JOB_START;
+	}
 
 	/* Preserve the timestamp on an existing unix print job */
 
-	old_pj = print_job_find(sharename, jobid);
+	old_pj = print_job_find(tmp_ctx, sharename, jobid);
 
 	ZERO_STRUCT(pj);
 
@@ -917,6 +933,7 @@ static void print_unix_job(struct tevent_context *ev,
 	fstrcpy(pj.queuename, old_pj ? old_pj->queuename : sharename );
 
 	pjob_store(ev, msg_ctx, sharename, jobid, &pj);
+	talloc_free(tmp_ctx);
 }
 
 
@@ -1348,8 +1365,9 @@ static void print_queue_update_internal(struct tevent_context *ev,
 	TDB_DATA jcdata;
 	fstring keystr, cachestr;
 	struct tdb_print_db *pdb = get_print_db_byname(sharename);
+	TALLOC_CTX *tmp_ctx = talloc_new(ev);
 
-	if (!pdb) {
+	if ((pdb == NULL) || (tmp_ctx == NULL)) {
 		return;
 	}
 
@@ -1390,7 +1408,6 @@ static void print_queue_update_internal(struct tevent_context *ev,
 
 	  fill in any system job numbers as we go
 	*/
-
 	jcdata = get_jobs_added_data(pdb);
 
 	for (i=0; i<qcount; i++) {
@@ -1403,7 +1420,7 @@ static void print_queue_update_internal(struct tevent_context *ev,
 		}
 
 		/* we have an active SMB print job - update its status */
-		pjob = print_job_find(sharename, jobid);
+		pjob = print_job_find(tmp_ctx, sharename, jobid);
 		if (!pjob) {
 			/* err, somethings wrong. Probably smbd was restarted
 			   with jobs in the queue. All we can do is treat them
@@ -1426,6 +1443,7 @@ static void print_queue_update_internal(struct tevent_context *ev,
 	}
 
 	SAFE_FREE(jcdata.dptr);
+	talloc_free(tmp_ctx);
 
 	/* now delete any queued entries that don't appear in the
            system queue */
@@ -1954,7 +1972,7 @@ bool print_job_exists(const char* sharename, uint32 jobid)
 
 char *print_job_fname(const char* sharename, uint32 jobid)
 {
-	struct printjob *pjob = print_job_find(sharename, jobid);
+	struct printjob *pjob = print_job_find(NULL, sharename, jobid);
 	if (!pjob || pjob->spooled || pjob->pid != getpid())
 		return NULL;
 	return pjob->filename;
@@ -1967,12 +1985,14 @@ char *print_job_fname(const char* sharename, uint32 jobid)
  has not been spooled.
 ****************************************************************************/
 
-struct spoolss_DeviceMode *print_job_devmode(const char* sharename, uint32 jobid)
+struct spoolss_DeviceMode *print_job_devmode(TALLOC_CTX *mem_ctx,
+					     const char *sharename,
+					     uint32 jobid)
 {
-	struct printjob *pjob = print_job_find(sharename, jobid);
+	struct printjob *pjob = print_job_find(mem_ctx, sharename, jobid);
-
+	if (pjob == NULL) {
-	if ( !pjob )
 		return NULL;
+	}
 
 	return pjob->devmode;
 }
@@ -1986,13 +2006,23 @@ bool print_job_set_name(struct tevent_context *ev,
 			const char *sharename, uint32 jobid, const char *name)
 {
 	struct printjob *pjob;
+	bool ret;
+	TALLOC_CTX *tmp_ctx = talloc_new(ev);
+	if (tmp_ctx == NULL) {
+		return false;
+	}
 
-	pjob = print_job_find(sharename, jobid);
+	pjob = print_job_find(tmp_ctx, sharename, jobid);
-	if (!pjob || pjob->pid != getpid())
+	if (!pjob || pjob->pid != getpid()) {
-		return False;
+		ret = false;
+		goto err_out;
+	}
 
 	fstrcpy(pjob->jobname, name);
-	return pjob_store(ev, msg_ctx, sharename, jobid, pjob);
+	ret = pjob_store(ev, msg_ctx, sharename, jobid, pjob);
+err_out:
+	talloc_free(tmp_ctx);
+	return ret;
 }
 
 /****************************************************************************
@@ -2003,17 +2033,12 @@ bool print_job_get_name(TALLOC_CTX *mem_ctx, const char *sharename, uint32_t job
 {
 	struct printjob *pjob;
 
-	pjob = print_job_find(sharename, jobid);
+	pjob = print_job_find(mem_ctx, sharename, jobid);
 	if (!pjob || pjob->pid != getpid()) {
 		return false;
 	}
 
-	*name = talloc_strdup(mem_ctx, pjob->jobname);
+	return pjob->jobname;
-	if (!*name) {
-		return false;
-	}
-
-	return true;
 }
 
 
@@ -2085,19 +2110,29 @@ static bool print_job_delete1(struct tevent_context *ev,
 			      int snum, uint32 jobid)
 {
 	const char* sharename = lp_const_servicename(snum);
-	struct printjob *pjob = print_job_find(sharename, jobid);
+	struct printjob *pjob;
 	int result = 0;
 	struct printif *current_printif = get_printer_fns( snum );
+	bool ret;
+	TALLOC_CTX *tmp_ctx = talloc_new(ev);
+	if (tmp_ctx == NULL) {
+		return false;
+	}
 
-	if (!pjob)
+	pjob = print_job_find(tmp_ctx, sharename, jobid);
-		return False;
+	if (!pjob) {
+		ret = false;
+		goto err_out;
+	}
 
 	/*
 	 * If already deleting just return.
 	 */
 
-	if (pjob->status == LPQ_DELETING)
+	if (pjob->status == LPQ_DELETING) {
-		return True;
+		ret = true;
+		goto err_out;
+	}
 
 	/* Hrm - we need to be able to cope with deleting a job before it
 	   has reached the spooler.  Just mark it as LPQ_DELETING and
@@ -2127,8 +2162,10 @@ static bool print_job_delete1(struct tevent_context *ev,
 			struct tdb_print_db *pdb = get_print_db_byname(sharename);
 			int njobs = 1;
 
-			if (!pdb)
+			if (!pdb) {
-				return False;
+				ret = false;
+				goto err_out;
+			}
 			pjob_delete(ev, msg_ctx, sharename, jobid);
 			/* Ensure we keep a rough count of the number of total jobs... */
 			tdb_change_int32_atomic(pdb->tdb, "INFO/total_jobs", &njobs, -1);
@@ -2138,7 +2175,10 @@ static bool print_job_delete1(struct tevent_context *ev,
 
 	remove_from_jobs_added( sharename, jobid );
 
-	return (result == 0);
+	ret = (result == 0);
+err_out:
+	talloc_free(tmp_ctx);
+	return ret;
 }
 
 /****************************************************************************
@@ -2149,12 +2189,23 @@ static bool is_owner(const struct auth_session_info *server_info,
 		     const char *servicename,
 		     uint32 jobid)
 {
-	struct printjob *pjob = print_job_find(servicename, jobid);
+	struct printjob *pjob;
+	bool ret;
+	TALLOC_CTX *tmp_ctx = talloc_new(server_info);
+	if (tmp_ctx == NULL) {
+		return false;
+	}
 
-	if (!pjob || !server_info)
+	pjob = print_job_find(tmp_ctx, servicename, jobid);
-		return False;
+	if (!pjob || !server_info) {
+		ret = false;
+		goto err_out;
+	}
 
-	return strequal(pjob->user, server_info->unix_info->sanitized_username);
+	ret = strequal(pjob->user, server_info->unix_info->sanitized_username);
+err_out:
+	talloc_free(tmp_ctx);
+	return ret;
 }
 
 /****************************************************************************
@@ -2168,7 +2219,11 @@ WERROR print_job_delete(const struct auth_session_info *server_info,
 	const char* sharename = lp_const_servicename(snum);
 	struct printjob *pjob;
 	bool 	owner;
-	char 	*fname;
+	WERROR werr;
+	TALLOC_CTX *tmp_ctx = talloc_new(msg_ctx);
+	if (tmp_ctx == NULL) {
+		return WERR_NOT_ENOUGH_MEMORY;
+	}
 
 	owner = is_owner(server_info, lp_const_servicename(snum), jobid);
 
@@ -2188,7 +2243,8 @@ pause, or resume print job. User name: %s. Printer name: %s.",
 			      lp_printername(snum) );
 		/* END_ADMIN_LOG */
 
-		return WERR_ACCESS_DENIED;
+		werr = WERR_ACCESS_DENIED;
+		goto err_out;
 	}
 
 	/*
@@ -2198,18 +2254,20 @@ pause, or resume print job. User name: %s. Printer name: %s.",
 	 * spool file & return.
 	 */
 
-	fname = print_job_fname(sharename, jobid);
+	pjob = print_job_find(tmp_ctx, sharename, jobid);
-	if (fname != NULL) {
+	if (!pjob || pjob->spooled || pjob->pid != getpid()) {
-		/* remove the spool file */
+		DEBUG(10, ("Skipping spool file removal for job %u\n", jobid));
-		DEBUG(10, ("print_job_delete: "
+	} else {
-			   "Removing spool file [%s]\n", fname));
+		DEBUG(10, ("Removing spool file [%s]\n", pjob->filename));
-		if (unlink(fname) == -1) {
+		if (unlink(pjob->filename) == -1) {
-			return map_werror_from_unix(errno);
+			werr = map_werror_from_unix(errno);
+			goto err_out;
 		}
 	}
 
 	if (!print_job_delete1(server_event_context(), msg_ctx, snum, jobid)) {
-		return WERR_ACCESS_DENIED;
+		werr = WERR_ACCESS_DENIED;
+		goto err_out;
 	}
 
 	/* force update the database and say the delete failed if the
@@ -2217,12 +2275,16 @@ pause, or resume print job. User name: %s. Printer name: %s.",
 
 	print_queue_update(msg_ctx, snum, True);
 
-	pjob = print_job_find(sharename, jobid);
+	pjob = print_job_find(tmp_ctx, sharename, jobid);
 	if (pjob && (pjob->status != LPQ_DELETING)) {
-		return WERR_ACCESS_DENIED;
+		werr = WERR_ACCESS_DENIED;
+		goto err_out;
 	}
+	werr = WERR_PRINTER_HAS_JOBS_QUEUED;
 
-	return WERR_PRINTER_HAS_JOBS_QUEUED;
+err_out:
+	talloc_free(tmp_ctx);
+	return werr;
 }
 
 /****************************************************************************
@@ -2238,9 +2300,12 @@ WERROR print_job_pause(const struct auth_session_info *server_info,
 	int ret = -1;
 	struct printif *current_printif = get_printer_fns( snum );
 	WERROR werr;
+	TALLOC_CTX *tmp_ctx = talloc_new(msg_ctx);
+	if (tmp_ctx == NULL) {
+		return WERR_NOT_ENOUGH_MEMORY;
+	}
 
-	pjob = print_job_find(sharename, jobid);
+	pjob = print_job_find(tmp_ctx, sharename, jobid);
-
 	if (!pjob || !server_info) {
 		DEBUG(10, ("print_job_pause: no pjob or user for jobid %u\n",
 			(unsigned int)jobid ));
@@ -2291,6 +2356,7 @@ pause, or resume print job. User name: %s. Printer name: %s.",
 	/* how do we tell if this succeeded? */
 	werr = WERR_OK;
 err_out:
+	talloc_free(tmp_ctx);
 	return werr;
 }
 
@@ -2307,9 +2373,11 @@ WERROR print_job_resume(const struct auth_session_info *server_info,
 	int ret;
 	struct printif *current_printif = get_printer_fns( snum );
 	WERROR werr;
+	TALLOC_CTX *tmp_ctx = talloc_new(msg_ctx);
+	if (tmp_ctx == NULL)
+		return WERR_NOT_ENOUGH_MEMORY;
 
-	pjob = print_job_find(sharename, jobid);
+	pjob = print_job_find(tmp_ctx, sharename, jobid);
-
 	if (!pjob || !server_info) {
 		DEBUG(10, ("print_job_resume: no pjob or user for jobid %u\n",
 			(unsigned int)jobid ));
@@ -2357,6 +2425,7 @@ pause, or resume print job. User name: %s. Printer name: %s.",
 
 	werr = WERR_OK;
 err_out:
+	talloc_free(tmp_ctx);
 	return werr;
 }
 
@@ -2371,26 +2440,36 @@ ssize_t print_job_write(struct tevent_context *ev,
 	const char* sharename = lp_const_servicename(snum);
 	ssize_t return_code;
 	struct printjob *pjob;
-
+	TALLOC_CTX *tmp_ctx = talloc_new(ev);
-	pjob = print_job_find(sharename, jobid);
+	if (tmp_ctx == NULL) {
-
-	if (!pjob)
 		return -1;
+	}
+
+	pjob = print_job_find(tmp_ctx, sharename, jobid);
+	if (!pjob) {
+		return_code = -1;
+		goto err_out;
+	}
+
 	/* don't allow another process to get this info - it is meaningless */
-	if (pjob->pid != getpid())
+	if (pjob->pid != getpid()) {
-		return -1;
+		return_code = -1;
+		goto err_out;
+	}
 
 	/* if SMBD is spooling this can't be allowed */
 	if (pjob->status == PJOB_SMBD_SPOOLING) {
-		return -1;
+		return_code = -1;
+		goto err_out;
 	}
 
 	return_code = write_data(pjob->fd, buf, size);
-
+	if (return_code > 0) {
-	if (return_code>0) {
 		pjob->size += size;
 		pjob_store(ev, msg_ctx, sharename, jobid, pjob);
 	}
+err_out:
+	talloc_free(tmp_ctx);
 	return return_code;
 }
 
@@ -2800,16 +2879,24 @@ void print_job_endpage(struct messaging_context *msg_ctx,
 {
 	const char* sharename = lp_const_servicename(snum);
 	struct printjob *pjob;
+	TALLOC_CTX *tmp_ctx = talloc_new(msg_ctx);
+	if (tmp_ctx == NULL) {
+		return;
+	}
 
-	pjob = print_job_find(sharename, jobid);
+	pjob = print_job_find(tmp_ctx, sharename, jobid);
-	if (!pjob)
+	if (!pjob) {
-		return;
+		goto err_out;
+	}
 	/* don't allow another process to get this info - it is meaningless */
-	if (pjob->pid != getpid())
+	if (pjob->pid != getpid()) {
-		return;
+		goto err_out;
+	}
 
 	pjob->page_count++;
 	pjob_store(server_event_context(), msg_ctx, sharename, jobid, pjob);
+err_out:
+	talloc_free(tmp_ctx);
 }
 
 /****************************************************************************
@@ -2825,17 +2912,22 @@ NTSTATUS print_job_end(struct messaging_context *msg_ctx, int snum,
 	struct printjob *pjob;
 	int ret;
 	SMB_STRUCT_STAT sbuf;
-	struct printif *current_printif = get_printer_fns( snum );
+	struct printif *current_printif = get_printer_fns(snum);
 	NTSTATUS status = NT_STATUS_UNSUCCESSFUL;
+	TALLOC_CTX *tmp_ctx = talloc_new(msg_ctx);
+	if (tmp_ctx == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
 
-	pjob = print_job_find(sharename, jobid);
+	pjob = print_job_find(tmp_ctx, sharename, jobid);
-
 	if (!pjob) {
-		return NT_STATUS_PRINT_CANCELLED;
+		status = NT_STATUS_PRINT_CANCELLED;
+		goto err_out;
 	}
 
 	if (pjob->spooled || pjob->pid != getpid()) {
-		return NT_STATUS_ACCESS_DENIED;
+		status = NT_STATUS_ACCESS_DENIED;
+		goto err_out;
 	}
 
 	if (close_type == NORMAL_CLOSE || close_type == SHUTDOWN_CLOSE) {
@@ -2915,6 +3007,8 @@ fail:
 	pjob->fd = -1;
 	unlink(pjob->filename);
 	pjob_delete(server_event_context(), msg_ctx, sharename, jobid);
+err_out:
+	talloc_free(tmp_ctx);
 	return status;
 }
 
@@ -2937,6 +3031,10 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx,
 	int max_reported_jobs = lp_max_reported_jobs(snum);
 	bool ret = False;
 	const char* sharename = lp_servicename(snum);
+	TALLOC_CTX *tmp_ctx = talloc_new(msg_ctx);
+	if (tmp_ctx == NULL) {
+		return false;
+	}
 
 	/* make sure the database is up to date */
 	if (print_cache_expired(lp_const_servicename(snum), True))
@@ -3003,7 +3101,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx,
 
 		jobid = IVAL(cgdata.dptr, i*4);
 		DEBUG(5,("get_stored_queue_info: added job = %u\n", (unsigned int)jobid));
-		pjob = print_job_find(lp_const_servicename(snum), jobid);
+		pjob = print_job_find(tmp_ctx, lp_const_servicename(snum), jobid);
 		if (!pjob) {
 			DEBUG(5,("get_stored_queue_info: failed to find added job = %u\n", (unsigned int)jobid));
 			remove_from_jobs_added(sharename, jobid);
@@ -3019,6 +3117,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx,
 		fstrcpy(queue[total_count].fs_user, pjob->user);
 		fstrcpy(queue[total_count].fs_file, pjob->jobname);
 		total_count++;
+		talloc_free(pjob);
 	}
 
 	/* Update the changed jobids. */
@@ -3040,7 +3139,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx,
 			DEBUG(5,("get_stored_queue_info: changed job: %u\n",
 				 (unsigned int) jobid));
 
-			pjob = print_job_find(sharename, jobid);
+			pjob = print_job_find(tmp_ctx, sharename, jobid);
 			if (pjob == NULL) {
 				DEBUG(5,("get_stored_queue_info: failed to find "
 					 "changed job = %u\n",
@@ -3057,6 +3156,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx,
 			queue[j].time = pjob->starttime;
 			fstrcpy(queue[j].fs_user, pjob->user);
 			fstrcpy(queue[j].fs_file, pjob->jobname);
+			talloc_free(pjob);
 
 			DEBUG(5,("get_stored_queue_info: updated queue[%u], jobid: %u, jobname: %s\n",
 				 (unsigned int) j, (unsigned int) jobid, pjob->jobname));
@@ -3084,6 +3184,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx,
 
 	SAFE_FREE(data.dptr);
 	SAFE_FREE(cgdata.dptr);
+	talloc_free(tmp_ctx);
 	return ret;
 }