In source3/printing/printspoolss.c print_spool_open() we always overwrite fsp->conn->spoolss_pipe, which introduces a memory leak. While trying to fix the memory leaks and use fsp->print_file->spoolss_pipe instead of fsp->conn->spoolss_pipe, I noticed that we call spoolss_OpenPrinter() and spoolss_StartDocPrinter() in print_spool_open(). In print_spool_write() we only write the given bytes to the tempfile. In print_spool_end() we spoolss_ClosePrinter(), with a comment that this implicitly calls spoolss_EndDocPrinter(). But we never seem to call spoolss_WritePrinter() with the content from the tempfile anywhere.
I think it works. smb2_create calls print_spool_open(). Inside print_spool_open() we create a filename relative to the root of the print share path via and open it as a file descriptor using: 120 pf->filename = talloc_asprintf(pf, "%s/%sXXXXXX", 121 lp_path(talloc_tos(), 122 SNUM(fsp->conn)), 123 PRINT_SPOOL_PREFIX); 124 if (!pf->filename) { 125 status = NT_STATUS_NO_MEMORY; 126 goto done; 127 } 128 errno = 0; 129 mask = umask(S_IRWXO | S_IRWXG); 130 fd = mkstemp(pf->filename); pf->filename is set into the DocumentInfo struct and passed into _spoolss_StartDocPrinter() here: 179 info1 = talloc(tmp_ctx, struct spoolss_DocumentInfo1); 180 if (info1 == NULL) { 181 status = NT_STATUS_NO_MEMORY; 182 goto done; 183 } 184 info1->document_name = pf->docname; 185 info1->output_file = pf->filename; 186 info1->datatype = "RAW"; 187 188 info_ctr.level = 1; 189 info_ctr.info.info1 = info1; 190 191 status = dcerpc_spoolss_StartDocPrinter(b, tmp_ctx, 192 &pf->handle, 193 &info_ctr, 194 &pf->jobid, 195 &werr); ... and then sets up as the target for normal writes into the returned (fd) handle via: 212 /* setup a full fsp */ 213 fsp->fsp_name = synthetic_smb_fname(fsp, pf->filename, NULL, NULL, 0); 214 if (fsp->fsp_name == NULL) { 215 status = NT_STATUS_NO_MEMORY; 216 goto done; 217 } 218 219 if (sys_fstat(fd, &fsp->fsp_name->st, false) != 0) { 220 status = map_nt_error_from_unix(errno); 221 goto done; 222 } 223 224 fsp->file_id = vfs_file_id_from_sbuf(fsp->conn, &fsp->fsp_name->st); 225 fsp->fh->fd = fd; Inside _spoolss_StartDocPrinter() we have: 5943 werr = print_job_start(p->session_info, 5944 p->msg_ctx, 5945 rhost, 5946 snum, 5947 info_1->document_name, 5948 info_1->output_file, 5949 Printer->devmode, 5950 &Printer->jobid); info_1->output_file is the open file we'll be writing into via the fd stored in fsp->fh->fd = fd inside print_spool_open(). print_job_start() allocates the jobid in the db, then calls: 2872 /* we have a job entry - now create the spool file */ 2873 werr = print_job_spool_file(snum, jobid, filename, &pjob); where filename == info_1->output_file above: print_job_spool_file() does the following - given filename==info1->output_file==output_file parameter below: static WERROR print_job_spool_file(int snum, uint32_t jobid, const char *output_file, struct printjob *pjob) { WERROR werr; SMB_STRUCT_STAT st; const char *path; int len; mode_t mask; /* if this file is within the printer path, it means that smbd * is spooling it and will pass us control when it is finished. * Verify that the file name is ok, within path, and it is * already already there */ if (output_file) { path = lp_path(talloc_tos(), snum); len = strlen(path); if (strncmp(output_file, path, len) == 0 && (output_file[len - 1] == '/' || output_file[len] == '/')) { /* verify path is not too long */ if (strlen(output_file) >= sizeof(pjob->filename)) { return WERR_INVALID_NAME; } /* verify that the file exists */ if (sys_stat(output_file, &st, false) != 0) { return WERR_INVALID_NAME; } fstrcpy(pjob->filename, output_file); DEBUG(3, ("print_job_spool_file:" "External spooling activated\n")); /* we do not open the file until spooling is done */ pjob->fd = -1; pjob->status = PJOB_SMBD_SPOOLING; return WERR_OK; } } ... When the file is closed, the close code calls print_spool_end() -> _spoolss_EndDocPrinter() -> print_job_end() where we have: 2964 if (close_type == NORMAL_CLOSE || close_type == SHUTDOWN_CLOSE) { 2965 if (pjob->status == PJOB_SMBD_SPOOLING) { 2966 /* take over the file now, smbd is done */ 2967 if (sys_stat(pjob->filename, &sbuf, false) != 0) { 2968 status = map_nt_error_from_unix(errno); 2969 DEBUG(3, ("print_job_end: " 2970 "stat file failed for jobid %d\n", 2971 jobid)); 2972 goto fail; 2973 } 2974 2975 pjob->status = LPQ_SPOOLING; 2976 pjob->filename contains the data written into the file via normal write calls. print_job_end() eventually passes over to the backend via: 3038 ret = (*(current_printif->job_submit))(snum, pjob, 3039 current_printif->type, lpq_cmd); So I think this still works (haven't tested, just analysed the code paths :-). Jeremy.
(In reply to Jeremy Allison from comment #1) Thanks! That explains it, but it's horrible :-) It seems that smbspool makes use of it, so we should be able to add tests for it. Once we have tests we should be able to refactor the code to use spoolss_WritePrinter() from smbd and avoid the shared file hack.
WIP :-) https://git.cryptomilk.org/users/asn/uid_wrapper.git/log/?h=master-init https://git.samba.org/?p=asn/samba.git;a=shortlog;h=refs/heads/master-smbspool
Andreas / printingmaintainers: what is the status of this?