Re: [sane.ds try2 1/3] Get resolution from sane, instead of hard coding -1.

[Juan Lang]

Hi Jeremy, a couple comments:

-        pImageInfo->XResolution.Whole = -1;
+        if (sane_option_get_int(activeDS.deviceHandle, "resolution",
&resolution) == SANE_STATUS_GOOD)
+            pImageInfo->XResolution.Whole =
pImageInfo->YResolution.Whole = resolution;
(snip)
+/* Option functions */
+#ifdef SONAME_LIBSANE
+SANE_Status sane_option_get_int(SANE_Handle h, const char
*option_name, SANE_Int *val);
+#endif

With this change, sane.ds will not compile unless SONAME_LIBSANE is
defined.  Is this intended?  More likely not, and you need to provide
a version of sane_option_get_int when it isn't defined, or use an
#ifdef at the call site (ick.)

The other comment is, is adding a new file (option.c) really
necessary?  If you're planning to expand it a lot, perhaps, but just
for this one small function it looks like overkill to me.
--Juan

[sane.ds try2 1/3] Get resolution from sane, instead of hard coding -1.

[Juan Lang]

Hi again, Jeremy,
> With this change, sane.ds will not compile unless SONAME_LIBSANE is
> defined.

This remark is incorrect, sorry.  I only read the patch, I didn't read
ds_image.c first.  This change is correct.
> The other comment is, is adding a new file (option.c) really
> necessary?  If you're planning to expand it a lot, perhaps, but just
> for this one small function it looks like overkill to me.

This remark still stands.
--Juan

[sane.ds try2 1/3] Get resolution from sane, instead of hard coding -1.

[Jeremy White]

Hi Juan,
> > The other comment is, is adding a new file (option.c) really
> > necessary?  If you're planning to expand it a lot, perhaps, but just
> > for this one small function it looks like overkill to me.
>
> This remark still stands.

Yes, I am planning on expanding options.c a fair amount,
and I'm going to need that functionality in at least 2 of the existing
files in sane.ds, so I thought it should go in another file.

Cheers,

Jeremy