Attachment 9429 Details for Bug 10271 – proposed fix for master branch

[patch] proposed fix for master branch

bso10271_fix.patch (text/plain), 8.44 KB, created by David Disseldorp on 2013-11-15 19:09:49 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Disseldorp

Created: 2013-11-15 19:09:49 UTC

Size: 8.44 KB

patch

obsolete

>From ec33367b253343fc8ed52b57f77436f665705b73 Mon Sep 17 00:00:00 2001
>From: David Disseldorp <ddiss@samba.org>
>Date: Thu, 19 Sep 2013 20:31:37 -0700
>Subject: [PATCH 1/2] spoolss: return the spoolss job ID in notifications
>
>Print job notifications currently carry the system print job identifier
>from the queue structure. Instead, the spoolss job identifier should be
>resolved and returned.
>
>Print clients can use notification job-ids in subsequent spoolss SetJob
>requests. Returning an incorrect identifier can result in the failure of
>such requests, e.g. spoolss_SetJob(SPOOLSS_JOB_CONTROL_DELETE).
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=10271
>
>Signed-off-by: David Disseldorp <ddiss@samba.org>
>---
> source3/include/printing.h                  |  1 +
> source3/printing/printing.c                 |  2 +-
> source3/rpc_server/spoolss/srv_spoolss_nt.c | 32 +++++++++++++++++++++++------
> 3 files changed, 28 insertions(+), 7 deletions(-)
>
>diff --git a/source3/include/printing.h b/source3/include/printing.h
>index 391fb7a..ec5a53b 100644
>--- a/source3/include/printing.h
>+++ b/source3/include/printing.h
>@@ -193,6 +193,7 @@ uint16_t print_spool_rap_jobid(struct print_file_data *print_file);
> 
> /* The following definitions come from printing/printing.c  */
> 
>+uint32 sysjob_to_jobid_pdb(struct tdb_print_db *pdb, int sysjob);
> uint32 sysjob_to_jobid(int unix_jobid);
> bool print_notify_register_pid(int snum);
> bool print_notify_deregister_pid(int snum);
>diff --git a/source3/printing/printing.c b/source3/printing/printing.c
>index 43f75e5..57d2f0c 100644
>--- a/source3/printing/printing.c
>+++ b/source3/printing/printing.c
>@@ -519,7 +519,7 @@ static int unixjob_traverse_fn(TDB_CONTEXT *the_tdb, TDB_DATA key,
> 	return 0;
> }
> 
>-static uint32 sysjob_to_jobid_pdb(struct tdb_print_db *pdb, int sysjob)
>+uint32 sysjob_to_jobid_pdb(struct tdb_print_db *pdb, int sysjob)
> {
> 	struct unixjob_traverse_state state;
> 
>diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c
>index d37c24d..a6201d4 100644
>--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c
>+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c
>@@ -3612,6 +3612,7 @@ static WERROR printer_notify_info(struct pipes_struct *p,
> 	print_status_struct status;
> 	struct spoolss_PrinterInfo2 *pinfo2 = NULL;
> 	WERROR result;
>+	struct tdb_print_db *pdb;
> 
> 	DEBUG(4,("printer_notify_info\n"));
> 
>@@ -3635,13 +3636,19 @@ static WERROR printer_notify_info(struct pipes_struct *p,
> 		return WERR_BADFID;
> 	}
> 
>+	pdb = get_print_db_byname(Printer->sharename);
>+	if (pdb == NULL) {
>+		return WERR_BADFID;
>+	}
>+
> 	/* Maybe we should use the SYSTEM session_info here... */
> 	result = winreg_get_printer_internal(mem_ctx,
> 				    get_session_info_system(),
> 				    p->msg_ctx,
> 				    lp_servicename(talloc_tos(), snum), &pinfo2);
> 	if (!W_ERROR_IS_OK(result)) {
>-		return WERR_BADFID;
>+		result = WERR_BADFID;
>+		goto err_pdb_drop;
> 	}
> 
> 	/*
>@@ -3650,10 +3657,11 @@ static WERROR printer_notify_info(struct pipes_struct *p,
> 	 */
> 	pinfo2->servername = talloc_strdup(pinfo2, Printer->servername);
> 	if (pinfo2->servername == NULL) {
>-		return WERR_NOMEM;
>+		result = WERR_NOMEM;
>+		goto err_pdb_drop;
> 	}
> 
>-	for (i=0; i<option->count; i++) {
>+	for (i = 0; i < option->count; i++) {
> 		option_type = option->types[i];
> 
> 		switch (option_type.type) {
>@@ -3672,12 +3680,21 @@ static WERROR printer_notify_info(struct pipes_struct *p,
> 			count = print_queue_status(p->msg_ctx, snum, &queue,
> 						   &status);
> 
>-			for (j=0; j<count; j++) {
>+			for (j = 0; j < count; j++) {
>+				uint32_t jobid;
>+				jobid = sysjob_to_jobid_pdb(pdb,
>+							    queue[j].sysjob);
>+				if (jobid == (uint32_t)-1) {
>+					DEBUG(2, ("ignoring untracked job %d\n",
>+						  queue[j].sysjob));
>+					continue;
>+				}
>+				/* FIXME check return value */
> 				construct_notify_jobs_info(p->msg_ctx,
> 							   &queue[j], info,
> 							   pinfo2, snum,
> 							   &option_type,
>-							   queue[j].sysjob,
>+							   jobid,
> 							   mem_ctx);
> 			}
> 
>@@ -3702,7 +3719,10 @@ static WERROR printer_notify_info(struct pipes_struct *p,
> 	*/
> 
> 	talloc_free(pinfo2);
>-	return WERR_OK;
>+	result = WERR_OK;
>+err_pdb_drop:
>+	release_print_db(pdb);
>+	return result;
> }
> 
> /****************************************************************
>-- 
>1.8.1.4
>
>
>From a0014f1f587c0ef6adf152b7be3d51a00588c955 Mon Sep 17 00:00:00 2001
>From: David Disseldorp <ddiss@samba.org>
>Date: Fri, 18 Oct 2013 13:09:23 +0200
>Subject: [PATCH 2/2] printing: always store sytem job-ID in queue state
>
>Print jobs have multiple identifiers: the regular spoolss jobid, which
>is allocated by spoolss on job submission, and the system jobid, which
>is assigned by the printing back-end.
>Currently these identifiers are incorrectly mixed in print job queue
>tracking. Fix this by ensuring that only the system jobid is stored in
>the print queue state structure.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=10271
>
>Signed-off-by: David Disseldorp <ddiss@samba.org>
>---
> source3/printing/printing.c | 43 ++++++++++++++++++++++---------------------
> 1 file changed, 22 insertions(+), 21 deletions(-)
>
>diff --git a/source3/printing/printing.c b/source3/printing/printing.c
>index 57d2f0c..b126bd5 100644
>--- a/source3/printing/printing.c
>+++ b/source3/printing/printing.c
>@@ -3051,7 +3051,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx,
> 	size_t len = 0;
> 	uint32 i;
> 	int max_reported_jobs = lp_max_reported_jobs(snum);
>-	bool ret = False;
>+	bool ret = false;
> 	const char* sharename = lp_servicename(talloc_tos(), snum);
> 	TALLOC_CTX *tmp_ctx = talloc_new(msg_ctx);
> 	if (tmp_ctx == NULL) {
>@@ -3095,7 +3095,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx,
> 
> 	/* Retrieve the linearised queue data. */
> 
>-	for( i  = 0; i < qcount; i++) {
>+	for(i = 0; i < qcount; i++) {
> 		uint32 qjob, qsize, qpage_count, qstatus, qpriority, qtime;
> 		len += tdb_unpack(data.dptr + len, data.dsize - len, "ddddddff",
> 				&qjob,
>@@ -3117,7 +3117,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx,
> 	total_count = qcount;
> 
> 	/* Add new jobids to the queue. */
>-	for( i  = 0; i < extra_count; i++) {
>+	for (i = 0; i < extra_count; i++) {
> 		uint32 jobid;
> 		struct printjob *pjob;
> 
>@@ -3130,7 +3130,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx,
> 			continue;
> 		}
> 
>-		queue[total_count].sysjob = jobid;
>+		queue[total_count].sysjob = pjob->sysjob;
> 		queue[total_count].size = pjob->size;
> 		queue[total_count].page_count = pjob->page_count;
> 		queue[total_count].status = pjob->status;
>@@ -3145,32 +3145,31 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx,
> 	/* Update the changed jobids. */
> 	for (i = 0; i < changed_count; i++) {
> 		uint32_t jobid = IVAL(jcdata.dptr, i * 4);
>+		struct printjob *pjob;
> 		uint32_t j;
> 		bool found = false;
> 
>+		pjob = print_job_find(tmp_ctx, sharename, jobid);
>+		if (pjob == NULL) {
>+			DEBUG(5,("get_stored_queue_info: failed to find "
>+				 "changed job = %u\n",
>+				 (unsigned int)jobid));
>+			remove_from_jobs_changed(sharename, jobid);
>+			continue;
>+		}
>+
> 		for (j = 0; j < total_count; j++) {
>-			if (queue[j].sysjob == jobid) {
>+			if (queue[j].sysjob == pjob->sysjob) {
> 				found = true;
> 				break;
> 			}
> 		}
> 
> 		if (found) {
>-			struct printjob *pjob;
>-
> 			DEBUG(5,("get_stored_queue_info: changed job: %u\n",
>-				 (unsigned int) 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",
>-					 (unsigned int) jobid));
>-				remove_from_jobs_changed(sharename, jobid);
>-				continue;
>-			}
>+				 (unsigned int)jobid));
> 
>-			queue[j].sysjob = jobid;
>+			queue[j].sysjob = pjob->sysjob;
> 			queue[j].size = pjob->size;
> 			queue[j].page_count = pjob->page_count;
> 			queue[j].status = pjob->status;
>@@ -3180,8 +3179,10 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx,
> 			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));
>+			DEBUG(5,("updated queue[%u], jobid: %u, sysjob: %u, "
>+				 "jobname: %s\n",
>+				 (unsigned int)j, (unsigned int)jobid,
>+				 (unsigned int)queue[j].sysjob, pjob->jobname));
> 		}
> 
> 		remove_from_jobs_changed(sharename, jobid);
>@@ -3200,7 +3201,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx,
> 	*ppqueue = queue;
> 	*pcount = total_count;
> 
>-	ret = True;
>+	ret = true;
> 
>   out:
> 
>-- 
>1.8.1.4
>

Flags:

asn: review+
gd: review+
jra: review+

Actions: View

Attachments on bug 10271: 9429 | 9439 | 9440 | 9444