s3-printing: Made print_job_start more robust.

Explicitly return ntstatus errors instead of relying on elusive errno.
Split the function to make it easier to follow.

Signed-off-by: Jim McDonough <jmcd@samba.org>

4 files changed, 168 insertions, 95 deletions

diff --git a/source3/include/proto.h b/source3/include/proto.h
index 32ccc48e14..8912e5e46e 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -4759,8 +4759,9 @@ bool print_job_resume(struct auth_serversupplied_info *server_info, int snum,
 		      uint32 jobid, WERROR *errcode);
 ssize_t print_job_write(int snum, uint32 jobid, const char *buf, SMB_OFF_T pos, size_t size);
 int print_queue_length(int snum, print_status_struct *pstatus);
-uint32 print_job_start(struct auth_serversupplied_info *server_info, int snum,
-		       const char *jobname, struct spoolss_DeviceMode *devmode );
+WERROR print_job_start(struct auth_serversupplied_info *server_info,
+		       int snum, const char *docname, const char *filename,
+		       struct spoolss_DeviceMode *devmode, uint32_t *_jobid);
 void print_job_endpage(int snum, uint32 jobid);
 bool print_job_end(int snum, uint32 jobid, enum file_close_type close_type);
 int print_queue_status(int snum, 
diff --git a/source3/printing/printfsp.c b/source3/printing/printfsp.c
index 5bb662e10f..4d34315a54 100644
--- a/source3/printing/printfsp.c
+++ b/source3/printing/printfsp.c
@@ -30,9 +30,10 @@ NTSTATUS print_fsp_open(struct smb_request *req, connection_struct *conn,
 			uint16_t current_vuid, files_struct *fsp)
 {
 	const char *svcname = lp_const_servicename(SNUM(conn));
-	int jobid;
+	uint32_t jobid;
 	fstring name;
 	NTSTATUS status;
+	WERROR werr;
 
 	fstrcpy( name, "Remote Downlevel Document");
 	if (fname) {
@@ -44,16 +45,15 @@ NTSTATUS print_fsp_open(struct smb_request *req, connection_struct *conn,
 		fstrcat(name, p);
 	}
 
-	jobid = print_job_start(conn->server_info, SNUM(conn), name, NULL);
-	if (jobid == -1) {
-		status = map_nt_error_from_unix(errno);
-		return status;
+	werr = print_job_start(conn->server_info, SNUM(conn),
+				 name, fname, NULL, &jobid);
+	if (!W_ERROR_IS_OK(werr)) {
+		return werror_to_ntstatus(werr);
 	}
 
 	fsp->print_file = talloc(fsp, struct print_file_data);
 	if (!fsp->print_file) {
-		status = map_nt_error_from_unix(ENOMEM);
-		return status;
+		return NT_STATUS_NO_MEMORY;
 	}
 
 	/* Convert to RAP id. */
diff --git a/source3/printing/printing.c b/source3/printing/printing.c
index b2ba6c5799..ebb91876fc 100644
--- a/source3/printing/printing.c
+++ b/source3/printing/printing.c
@@ -2316,39 +2316,54 @@ int print_queue_length(int snum, print_status_struct *pstatus)
  Allocate a jobid. Hold the lock for as short a time as possible.
 ***************************************************************************/
 
-static bool allocate_print_jobid(struct tdb_print_db *pdb, int snum, const char *sharename, uint32 *pjobid)
+static WERROR allocate_print_jobid(struct tdb_print_db *pdb, int snum,
+				   const char *sharename, uint32 *pjobid)
 {
 	int i;
 	uint32 jobid;
+	enum TDB_ERROR terr;
+	int ret;
 
 	*pjobid = (uint32)-1;
 
 	for (i = 0; i < 3; i++) {
 		/* Lock the database - only wait 20 seconds. */
-		if (tdb_lock_bystring_with_timeout(pdb->tdb, "INFO/nextjob", 20) == -1) {
-			DEBUG(0,("allocate_print_jobid: failed to lock printing database %s\n", sharename));
-			return False;
+		ret = tdb_lock_bystring_with_timeout(pdb->tdb,
+						     "INFO/nextjob", 20);
+		if (ret == -1) {
+			DEBUG(0, ("allocate_print_jobid: "
+				  "Failed to lock printing database %s\n",
+				  sharename));
+			terr = tdb_error(pdb->tdb);
+			return ntstatus_to_werror(map_nt_error_from_tdb(terr));
 		}
 
 		if (!tdb_fetch_uint32(pdb->tdb, "INFO/nextjob", &jobid)) {
-			if (tdb_error(pdb->tdb) != TDB_ERR_NOEXIST) {
-				DEBUG(0, ("allocate_print_jobid: failed to fetch INFO/nextjob for print queue %s\n",
-					sharename));
+			terr = tdb_error(pdb->tdb);
+			if (terr != TDB_ERR_NOEXIST) {
+				DEBUG(0, ("allocate_print_jobid: "
+					  "Failed to fetch INFO/nextjob "
+					  "for print queue %s\n", sharename));
 				tdb_unlock_bystring(pdb->tdb, "INFO/nextjob");
-				return False;
+				return ntstatus_to_werror(map_nt_error_from_tdb(terr));
 			}
-			DEBUG(10,("allocate_print_jobid: no existing jobid in %s\n", sharename));
+			DEBUG(10, ("allocate_print_jobid: "
+				   "No existing jobid in %s\n", sharename));
 			jobid = 0;
 		}
 
-		DEBUG(10,("allocate_print_jobid: read jobid %u from %s\n", jobid, sharename));
+		DEBUG(10, ("allocate_print_jobid: "
+			   "Read jobid %u from %s\n", jobid, sharename));
 
 		jobid = NEXT_JOBID(jobid);
 
-		if (tdb_store_int32(pdb->tdb, "INFO/nextjob", jobid)==-1) {
-			DEBUG(3, ("allocate_print_jobid: failed to store INFO/nextjob.\n"));
+		ret = tdb_store_int32(pdb->tdb, "INFO/nextjob", jobid);
+		if (ret == -1) {
+			terr = tdb_error(pdb->tdb);
+			DEBUG(3, ("allocate_print_jobid: "
+				  "Failed to store INFO/nextjob.\n"));
 			tdb_unlock_bystring(pdb->tdb, "INFO/nextjob");
-			return False;
+			return ntstatus_to_werror(map_nt_error_from_tdb(terr));
 		}
 
 		/* We've finished with the INFO/nextjob lock. */
@@ -2357,15 +2372,16 @@ static bool allocate_print_jobid(struct tdb_print_db *pdb, int snum, const char
 		if (!print_job_exists(sharename, jobid)) {
 			break;
 		}
-		DEBUG(10,("allocate_print_jobid: found jobid %u in %s\n", jobid, sharename));
+		DEBUG(10, ("allocate_print_jobid: "
+			   "Found jobid %u in %s\n", jobid, sharename));
 	}
 
 	if (i > 2) {
-		DEBUG(0, ("allocate_print_jobid: failed to allocate a print job for queue %s\n",
-			sharename));
+		DEBUG(0, ("allocate_print_jobid: "
+			  "Failed to allocate a print job for queue %s\n",
+			  sharename));
 		/* Probably full... */
-		errno = ENOSPC;
-		return False;
+		return WERR_NO_SPOOL_SPACE;
 	}
 
 	/* Store a dummy placeholder. */
@@ -2376,14 +2392,16 @@ static bool allocate_print_jobid(struct tdb_print_db *pdb, int snum, const char
 		dum.dsize = 0;
 		if (tdb_store(pdb->tdb, print_key(jobid, &tmp), dum,
 			      TDB_INSERT) == -1) {
-			DEBUG(3, ("allocate_print_jobid: jobid (%d) failed to store placeholder.\n",
-				jobid ));
-			return False;
+			DEBUG(3, ("allocate_print_jobid: "
+				  "jobid (%d) failed to store placeholder.\n",
+				  jobid ));
+			terr = tdb_error(pdb->tdb);
+			return ntstatus_to_werror(map_nt_error_from_tdb(terr));
 		}
 	}
 
 	*pjobid = jobid;
-	return True;
+	return WERR_OK;
 }
 
 /***************************************************************************
@@ -2405,73 +2423,127 @@ static bool add_to_jobs_changed(struct tdb_print_db *pdb, uint32 jobid)
 			   data) == 0);
 }
 
+
 /***************************************************************************
- Start spooling a job - return the jobid.
+ Do all checks needed to determine if we can start a job.
 ***************************************************************************/
 
-uint32 print_job_start(struct auth_serversupplied_info *server_info, int snum,
-		       const char *jobname, struct spoolss_DeviceMode *devmode )
+static WERROR print_job_checks(struct auth_serversupplied_info *server_info,
+			       int snum, int *njobs)
 {
-	uint32 jobid;
-	char *path;
-	struct printjob pjob;
 	const char *sharename = lp_const_servicename(snum);
-	struct tdb_print_db *pdb = get_print_db_byname(sharename);
-	int njobs;
-
-	errno = 0;
-
-	if (!pdb)
-		return (uint32)-1;
+	uint64_t dspace, dsize;
+	uint64_t minspace;
+	int ret;
 
 	if (!print_access_check(server_info, snum, PRINTER_ACCESS_USE)) {
-		DEBUG(3, ("print_job_start: job start denied by security descriptor\n"));
-		release_print_db(pdb);
-		return (uint32)-1;
+		DEBUG(3, ("print_job_checks: "
+			  "job start denied by security descriptor\n"));
+		return WERR_ACCESS_DENIED;
 	}
 
-	if (!print_time_access_check(server_info, lp_servicename(snum))) {
-		DEBUG(3, ("print_job_start: job start denied by time check\n"));
-		release_print_db(pdb);
-		return (uint32)-1;
+	if (!print_time_access_check(server_info, sharename)) {
+		DEBUG(3, ("print_job_checks: "
+			  "job start denied by time check\n"));
+		return WERR_ACCESS_DENIED;
 	}
 
-	path = lp_pathname(snum);
-
 	/* see if we have sufficient disk space */
 	if (lp_minprintspace(snum)) {
-		uint64_t dspace, dsize;
-		if (sys_fsusage(path, &dspace, &dsize) == 0 &&
-		    dspace < 2*(uint64_t)lp_minprintspace(snum)) {
-			DEBUG(3, ("print_job_start: disk space check failed.\n"));
-			release_print_db(pdb);
-			errno = ENOSPC;
-			return (uint32)-1;
+		minspace = lp_minprintspace(snum);
+		ret = sys_fsusage(lp_pathname(snum), &dspace, &dsize);
+		if (ret == 0 && dspace < 2*minspace) {
+			DEBUG(3, ("print_job_checks: "
+				  "disk space check failed.\n"));
+			return WERR_NO_SPOOL_SPACE;
 		}
 	}
 
 	/* for autoloaded printers, check that the printcap entry still exists */
-	if (lp_autoloaded(snum) && !pcap_printername_ok(lp_const_servicename(snum))) {
-		DEBUG(3, ("print_job_start: printer name %s check failed.\n", lp_const_servicename(snum) ));
-		release_print_db(pdb);
-		errno = ENOENT;
-		return (uint32)-1;
+	if (lp_autoloaded(snum) && !pcap_printername_ok(sharename)) {
+		DEBUG(3, ("print_job_checks: printer name %s check failed.\n",
+			  sharename));
+		return WERR_ACCESS_DENIED;
 	}
 
 	/* Insure the maximum queue size is not violated */
-	if ((njobs = print_queue_length(snum,NULL)) > lp_maxprintjobs(snum)) {
-		DEBUG(3, ("print_job_start: Queue %s number of jobs (%d) larger than max printjobs per queue (%d).\n",
-			sharename, njobs, lp_maxprintjobs(snum) ));
+	*njobs = print_queue_length(snum, NULL);
+	if (*njobs > lp_maxprintjobs(snum)) {
+		DEBUG(3, ("print_job_checks: Queue %s number of jobs (%d) "
+			  "larger than max printjobs per queue (%d).\n",
+			  sharename, *njobs, lp_maxprintjobs(snum)));
+		return WERR_NO_SPOOL_SPACE;
+	}
+
+	return WERR_OK;
+}
+
+/***************************************************************************
+ Create a job file.
+***************************************************************************/
+
+static WERROR print_job_spool_file(int snum, uint32_t jobid,
+				   fstring *filename, int *fd)
+{
+	WERROR werr;
+
+	slprintf(*filename, sizeof(*filename)-1, "%s/%s%.8u.XXXXXX",
+		 lp_pathname(snum), PRINT_SPOOL_PREFIX, (unsigned int)jobid);
+	*fd = mkstemp(*filename);
+
+	if (*fd == -1) {
+		werr = map_werror_from_unix(errno);
+		if (W_ERROR_EQUAL(werr, WERR_ACCESS_DENIED)) {
+			/* Common setup error, force a report. */
+			DEBUG(0, ("print_job_spool_file: "
+				  "insufficient permissions to open spool "
+				  "file %s.\n", *filename));
+		} else {
+			/* Normal case, report at level 3 and above. */
+			DEBUG(3, ("print_job_spool_file: "
+				  "can't open spool file %s\n", *filename));
+		}
+		return werr;
+	}
+
+	return WERR_OK;
+}
+
+/***************************************************************************
+ Start spooling a job - return the jobid.
+***************************************************************************/
+
+WERROR print_job_start(struct auth_serversupplied_info *server_info,
+		       int snum, const char *docname, const char *filename,
+		       struct spoolss_DeviceMode *devmode, uint32_t *_jobid)
+{
+	uint32_t jobid;
+	char *path;
+	struct printjob pjob;
+	const char *sharename = lp_const_servicename(snum);
+	struct tdb_print_db *pdb = get_print_db_byname(sharename);
+	int njobs;
+	WERROR werr;
+
+	if (!pdb) {
+		return WERR_INTERNAL_DB_CORRUPTION;
+	}
+
+	path = lp_pathname(snum);
+
+	werr = print_job_checks(server_info, snum, &njobs);
+	if (!W_ERROR_IS_OK(werr)) {
 		release_print_db(pdb);
-		errno = ENOSPC;
-		return (uint32)-1;
+		return werr;
 	}
 
 	DEBUG(10,("print_job_start: Queue %s number of jobs (%d), max printjobs = %d\n",
 		sharename, njobs, lp_maxprintjobs(snum) ));
 
-	if (!allocate_print_jobid(pdb, snum, sharename, &jobid))
+	werr = allocate_print_jobid(pdb, snum, sharename, &jobid);
+	if (!W_ERROR_IS_OK(werr)) {
 		goto fail;
+	}
 
 	/* create the database entry */
 
@@ -2487,7 +2559,7 @@ uint32 print_job_start(struct auth_serversupplied_info *server_info, int snum,
 	pjob.smbjob = True;
 	pjob.devmode = devmode;
 
-	fstrcpy(pjob.jobname, jobname);
+	fstrcpy(pjob.jobname, docname);
 
 	fstrcpy(pjob.user, lp_printjob_username(snum));
 	standard_sub_advanced(sharename, server_info->sanitized_username,
@@ -2501,20 +2573,8 @@ uint32 print_job_start(struct auth_serversupplied_info *server_info, int snum,
 	fstrcpy(pjob.queuename, lp_const_servicename(snum));
 
 	/* we have a job entry - now create the spool file */
-	slprintf(pjob.filename, sizeof(pjob.filename)-1, "%s/%s%.8u.XXXXXX",
-		 path, PRINT_SPOOL_PREFIX, (unsigned int)jobid);
-	pjob.fd = mkstemp(pjob.filename);
-
-	if (pjob.fd == -1) {
-		if (errno == EACCES) {
-			/* Common setup error, force a report. */
-			DEBUG(0, ("print_job_start: insufficient permissions \
-to open spool file %s.\n", pjob.filename));
-		} else {
-			/* Normal case, report at level 3 and above. */
-			DEBUG(3, ("print_job_start: can't open spool file %s,\n", pjob.filename));
-			DEBUGADD(3, ("errno = %d (%s).\n", errno, strerror(errno)));
-		}
+	werr = print_job_spool_file(snum, jobid, &pjob.filename, &pjob.fd);
+	if (!W_ERROR_IS_OK(werr)) {
 		goto fail;
 	}
 
@@ -2528,16 +2588,19 @@ to open spool file %s.\n", pjob.filename));
 
 	release_print_db(pdb);
 
-	return jobid;
+	*_jobid = jobid;
+	return WERR_OK;
 
  fail:
-	if (jobid != -1)
+	if (jobid != -1) {
 		pjob_delete(sharename, jobid);
+	}
 
 	release_print_db(pdb);
 
-	DEBUG(3, ("print_job_start: returning fail. Error = %s\n", strerror(errno) ));
-	return (uint32)-1;
+	DEBUG(3, ("print_job_start: returning fail. "
+		  "Error = %s\n", win_errstr(werr)));
+	return werr;
 }
 
 /****************************************************************************
diff --git a/source3/rpc_server/srv_spoolss_nt.c b/source3/rpc_server/srv_spoolss_nt.c
index 86dffafb73..24d1716d42 100644
--- a/source3/rpc_server/srv_spoolss_nt.c
+++ b/source3/rpc_server/srv_spoolss_nt.c
@@ -5121,6 +5121,7 @@ WERROR _spoolss_StartDocPrinter(pipes_struct *p,
 	struct spoolss_DocumentInfo1 *info_1;
 	int snum;
 	Printer_entry *Printer = find_printer_index_by_hnd(p, r->in.handle);
+	WERROR werr;
 
 	if (!Printer) {
 		DEBUG(2,("_spoolss_StartDocPrinter: "
@@ -5129,6 +5130,13 @@ WERROR _spoolss_StartDocPrinter(pipes_struct *p,
 		return WERR_BADFID;
 	}
 
+	if (Printer->jobid) {
+		DEBUG(2, ("_spoolss_StartDocPrinter: "
+			  "StartDocPrinter called twice! "
+			  "(existing jobid = %d)\n", Printer->jobid));
+		return WERR_INVALID_HANDLE;
+	}
+
 	if (r->in.level != 1) {
 		return WERR_UNKNOWN_LEVEL;
 	}
@@ -5155,15 +5163,15 @@ WERROR _spoolss_StartDocPrinter(pipes_struct *p,
 		return WERR_BADFID;
 	}
 
-	Printer->jobid = print_job_start(p->server_info, snum,
-					 info_1->document_name,
-					 Printer->devmode);
+	werr = print_job_start(p->server_info, snum,
+				info_1->document_name, info_1->output_file,
+				Printer->devmode, &Printer->jobid);
 
 	/* An error occured in print_job_start() so return an appropriate
 	   NT error code. */
 
-	if (Printer->jobid == -1) {
-		return map_werror_from_unix(errno);
+	if (!W_ERROR_IS_OK(werr)) {
+		return werr;
 	}
 
 	Printer->document_started = true;
@@ -5196,6 +5204,7 @@ WERROR _spoolss_EndDocPrinter(pipes_struct *p,
 	print_job_end(snum, Printer->jobid, NORMAL_CLOSE);
 	/* error codes unhandled so far ... */
 
+	Printer->jobid = 0;
 	return WERR_OK;
 }