Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Samsung printer drivers incorrectly use Emulators keyword to configure themselves #5562

Closed
neiser opened this issue Apr 11, 2019 · 13 comments

Comments

@neiser
Copy link

commented Apr 11, 2019

As reported in the comments here https://aur.archlinux.org/pkgbase/samsung-unified-driver there's a regression when using the version 2.2.11 instead of 2.2.10. Instead of printing the file, it prints the error message

SPL-C ERROR - Please use the proper driver
POSITION : 0x0 (0)
SYSTEM : src_5.47/xl_image
LINE : 629
VERSION : SPL-C 5.47 01-15-2010

I've bisected the problem using my printer Samsung CLX-3185 and found the following commit to be the first commit introducing the regression 558bba7

Unfortunately, I cannot test the current master version if the regression still persists, as I'm getting a nasty compilation error after applying ArchLinux patches:

Compiling ippeveprinter.c...
ippeveprinter.c:163:3: error: unknown type name 'ippeve_srv_t'; did you mean
      'ippeve_job_t'?
  ippeve_srv_t          ipp_ref,        /* Bonjour IPP service */
  ^~~~~~~~~~~~
  ippeve_job_t
ippeveprinter.c:156:29: note: 'ippeve_job_t' declared here
typedef struct ippeve_job_s ippeve_job_t;
                            ^
ippeveprinter.c:163:17: error: field has incomplete type 'ippeve_job_t' (aka
      'struct ippeve_job_s')
  ippeve_srv_t          ipp_ref,        /* Bonjour IPP service */
                        ^
ippeveprinter.c:156:16: note: forward declaration of 'struct ippeve_job_s'
typedef struct ippeve_job_s ippeve_job_t;
               ^
ippeveprinter.c:164:4: error: field has incomplete type 'ippeve_job_t' (aka
      'struct ippeve_job_s')
                        ipps_ref,       /* Bonjour IPPS service */
                        ^
...

When I try to revert the problematic commit 558bba7 on v2.2.11, I get the following linking error:

Linking libcups.so.2...
clang-8: warning: argument unused during compilation: '-pie' [-Wunused-command-line-argument]
Making all in test...
Compiling ippfind.c...
Linking ippfind...
/usr/bin/ld: ../cups/libcups.so: undefined reference to `ppd_free'
clang-8: error: linker command failed with exit code 1 (use -v to see invocation)
make[1]: *** [Makefile:186: ippfind] Error 1
make: *** [Makefile:38: all] Error 1

Which I was able to fix by adding the line

#define ppd_free(p)	if (p) free(p)	/* Safe free macro */

back to cups/ppd.c

The finally build package with this "reverted" v2.2.11 did not show the regression, which pretty much indicates that 558bba7 is really the problem here (although the related PR #5475 told me that it just removed dead code, maybe not so dead at all...)

Maybe someone can help me fixing those compile errors on the master branch, or can manually check if the above commit has been reverted in the meantime.

I'm happy to provide more information on the problem or test proposed patches (if I get them to compile on my machine).

@adamuc

This comment has been minimized.

Copy link

commented Apr 13, 2019

A very similar thing occurring on my CLX-3170 too:

SPL-C ERROR - Please use the proper driver  

     POSITION : 0x0 (0)

     SYSTEM   : src/xl_image

     LINE     : 606

     VERSION  : SPL-C 5.35 11-20-2007
@liOnux33

This comment has been minimized.

Copy link

commented Apr 14, 2019

Same issue on my CLP-320 with CUPS-2.2.10-2 and libcups-2.2.11-1

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator

commented Apr 15, 2019

@neiser Try building master without the ArchLinux changes. The 2.3 build system has some configure changes that are probably tripping you up.

I'm not sure why the change to stop loading the emulators is causing a problem - nothing in CUPS ever used them. Perhaps if you attach the PPD file in question I can see what that driver is doing.

@liOnux33

This comment has been minimized.

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator

commented Apr 15, 2019

OK, so this is a driver issue - it is misusing the Emulators keyword as a driver setting. The fix (for all versions of CUPS) is to use the ppdFindAttr function to get the value instead.

@christophgysin

This comment has been minimized.

Copy link

commented Apr 15, 2019

@michaelrsweet Can you elaborate on your suggested fix?

Is there a way to patch the PPD files in question to make them work again without having to revert the above mentioned commit?

I understand the Samsung drivers are at fault. But all users with Samsung printers that previously worked with cups are now broken, because cups removed a feature that was not supposed to be used in the first place.

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator

commented Apr 15, 2019

@christophgysin There is no way to patch the PPDs. The filter needs to be fixed as it relied on undocumented and unsupported behavior. Reverting the bug fix is not a solution because you'll still have the memory leak bug, and since we don't use the Emulators keyword there is no point in trying to continue loading the array and making sure we don't leak memory.

@SteveFosdick

This comment has been minimized.

Copy link

commented Apr 15, 2019

It seems to me the way forwards is regard the use of the unsupported emulators feature by the Samsung driver as a reason for it to be a supported feature and fix the memory leak within the implementation of that feature rather than remove the feature.

Even before the memory leak is fixed a memory leak is much preferable to an unusable printer. The classes of printers supported by this driver include some expensive laser printers while memory is cheap.

@michaelrsweet michaelrsweet changed the title Regression: Error message printed with Samsung CLX-3185 and CLP-320 Samsung printer drivers incorrectly use Emulators keyword to configure themselves Apr 15, 2019

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator

commented Apr 15, 2019

@SteveFosdick While I sympathize with your situation, PPD files have been deprecated in CUPS for 10 years (and Samsung was at the PWG/OpenPrinting meeting when we announced it) and printer driver support is scheduled to be dropped after CUPS 2.3.x. Fixing the memory leak (as opposed to dropping support for the keyword) was going to be a major effort, and with no valid uses for the information (and no source code usage that we could find) the decision was made to drop it.

I will consider changing the fix to support a single Emulators value, which will provide a temporary fix (until drivers are dropped entirely) for this issue. However, you need to contact Samsung or HP to get a proper fix to their code.

@SteveFosdick

This comment has been minimized.

Copy link

commented Apr 15, 2019

Thanks, Michael.

michaelrsweet added a commit that referenced this issue Apr 15, 2019
@michaelrsweet

This comment has been minimized.

Copy link
Collaborator

commented Apr 15, 2019

[master 4c00fa5] Add a workaround for old Samsung drivers (Issue #5562)

[branch-2.2 8829edf] Add a workaround for old Samsung drivers (Issue #5562)

michaelrsweet added a commit that referenced this issue Apr 15, 2019
@neiser

This comment has been minimized.

Copy link
Author

commented Apr 17, 2019

@michaelrsweet Thanks a lot for adding a workaround and your quick response to this issue. I can fully understand that it's tedious to cope with outdated drivers from 3rd party vendors.

I can confirm that the current branch-2.2 works nicely with my Samsung printer again. So, thanks a lot!

@hannut

This comment has been minimized.

Copy link

commented May 9, 2019

I have this same problem with Fedora 30 and CLX-3170. Fedora 29 works. I'm using the negativo17 repository for samsung unified driver -package.

Cups versions:
Fedora 29: cups-2.2.8-10.fc29.x86_64
Fedora 30: cups-2.2.11-2.fc30.x86_64

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.