Bug 7288 - SMB job IDs in CUPS job names wrong
Summary: SMB job IDs in CUPS job names wrong
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.2
Classification: Unclassified
Component: Printing (show other bugs)
Version: 3.2.5
Hardware: Other Linux
: P3 normal
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-24 10:10 UTC by Michael Karcher
Modified: 2010-04-15 09:55 UTC (History)
1 user (show)

See Also:


Attachments
Patch that takes the job ID from the spool file name (1.62 KB, patch)
2010-03-24 10:13 UTC, Michael Karcher
no flags Details
Bouncing fingers on a google shuttle led me to submit this prematurely :-). (3.46 KB, application/octet-stream)
2010-03-25 19:30 UTC, Jeremy Allison
no flags Details
Patch I've applied to master. (3.46 KB, patch)
2010-03-25 19:35 UTC, Jeremy Allison
no flags Details
git-am format patch for 3.5.2 and 3.4.8 (4.71 KB, patch)
2010-03-25 19:46 UTC, Jeremy Allison
no flags Details
git am patch for 3.5.x and 3.4.x (5.00 KB, patch)
2010-03-25 20:25 UTC, Jeremy Allison
jra: review? (gd)
metze: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Karcher 2010-03-24 10:10:56 UTC
currently samba (ab)uses the smbjob field of the print job structure to pass the job ID to the CUPS backend. As this field is declared bool, and bool maps to _Bool on C99 compatible implementations, the job ID is demoted to the range of _Bool (i.e. 0 or 1), causing all print jobs to start with smbprn.00000001 and mixup of the jobs when the Queue is parsed. The attached patch fixes it.
Comment 1 Michael Karcher 2010-03-24 10:13:10 UTC
Created attachment 5532 [details]
Patch that takes the job ID from the spool file name
Comment 2 Jeremy Allison 2010-03-24 18:56:01 UTC
Looks correct. The abuse was from older Samba code where we used our own defined BOOL which was defined as an "int".
Jeremy.
Comment 3 Jeremy Allison 2010-03-25 14:14:35 UTC
Ok, I've looked at this carefully, and I think I'd like to rewrite it to use the existing function we have in printing/printing.c called:

print_parse_jobid(char *filename)

Modified patch to follow....

Jeremy.
Comment 4 Jeremy Allison 2010-03-25 19:30:38 UTC
Created attachment 5546 [details]
Bouncing fingers on a google shuttle led me to submit this prematurely :-).
Comment 5 Jeremy Allison 2010-03-25 19:35:19 UTC
Created attachment 5547 [details]
Patch I've applied to master.

I think this is the correct fix. It causes cups_job_submit to use print_parse_jobid(), which I've moved into printing/lpq_parse.c (to allow the link to work).

It turns out the old print_parse_jobid() was *broken*, in that the pjob filename was set as an absolute path - not relative to the sharename (due to it not going through the VFS calls).

This meant that the original code doing a strncmp on the first part of the filename would always fail - it starts with a "/", not the relative pathname of PRINT_SPOOL_PREFIX ("smbprn.").

This fix could fix some other mysterious printing bugs - probably the ones Guenther noticed where job control fails on non-cups backends.

Jeremy.
Comment 6 Jeremy Allison 2010-03-25 19:46:56 UTC
Created attachment 5548 [details]
git-am format patch for 3.5.2 and 3.4.8

Guenther please review and pass to Karolin if you're happy.
Jeremy.
Comment 7 Jeremy Allison 2010-03-25 20:25:43 UTC
Created attachment 5549 [details]
git am patch for 3.5.x and 3.4.x

Replacement fix - includes the removal of the setting of the smbjob bool as well as the rewrite of print_parse_jobid.

Guenther please check then reassign to Karolin for inclusion.

Jeremy.
Comment 8 Jeremy Allison 2010-04-13 12:30:12 UTC
Pinging Guenther for review (user reporting this on the mailing list).

CC:ing Karolin so this doesn't get missed for 3.5.x and 3.4.x.

Jeremy.
Comment 9 Jeremy Allison 2010-04-13 12:30:45 UTC
Comment on attachment 5549 [details]
git am patch for 3.5.x and 3.4.x

Requesting review from Metze in case Guenther is busy.

Jeremy.
Comment 10 Michael Karcher 2010-04-13 12:38:05 UTC
Just as a note: The original implementation (matching only at the beginning of the string) was working fine for lpd queues to parse the document filename, because traditionally, these file names were listed without paths, so parsing the queue entry names worked.
Comment 11 Stefan Metzmacher 2010-04-13 13:47:36 UTC
Comment on attachment 5549 [details]
git am patch for 3.5.x and 3.4.x

looks good
Comment 12 Stefan Metzmacher 2010-04-13 14:13:12 UTC
Karolin, please pick for the next releases
Comment 13 Karolin Seeger 2010-04-15 09:55:03 UTC
Pushed to both branches.
Closing out bug report.

Thanks!