Xapian devel - Dec 2014 - Replace atoi and atol with strtol strtoul:Need Help

On Thu, Dec 18, 2014 at 11:38:42PM +0000, James Aylett
wrote:
> Collapsing is done using a valueno (you can tell because collapse_key
> is of type Xapian::valueno), which is a 32 bit unsigned integer (see
> here:
>
https://getting-started-with-xapian.readthedocs.org/en/latest/concepts/indexing/values.html).
> For 32 bit unsigned you need unsigned long.
Xapian::valueno is actually a typedef for unsigned currently.  While the
standards permit 16 bit integers, no relevant platforms actually use
them nowadays, so pragmatically unsigned is a better choice if you need
to pick a single built-in type.

It'd be nice to be a bit smarter about this - now we've decided to
require C++11 for trunk, we can now use uint32_t here.  Prior to C++11
I think we'd have to generate the API header based on configure checks
and then install it to an architecture-dependent path.

But anyway, there's no "strtou()" so you have to converting to
unsigned
long with strtoul() and then check for potential overflow from the cast
to Xapian::valueno.
> I think you?ve been looking at other uses of atoi/atol; if you?ve
> successfully updated code then you should make a pull request or email
> a patch (or attach one to the trac ticket) so someone can review it
> and get it into trunk (and hence into a future release). Smaller PRs
> are generally easier to review than ones that touch lots of files, and
> it?s not a problem to have a number of small PRs, each fixing atoi()
> usage in one subsystem of Xapian.
Seconded.

Another reason to get some of your changes reviewed early is in case
there's something you're doing wrong - better to discover that before
you repeat it in a lot of different places.

Cheers,
    Olly

Hello,
As James said I am attaching two file *date.cc* and *datematchdecider.cc*
present in */xapian-application/omega/* in which i modified atoi function .
I have also modified some files but i first need to check whether i am
going in right direction. *Still I am not certain what to do when an
overflow or underflow condition's occurs so i have just kept it blank.*

I have checked for overflow and underflow condition using errno variable
which becomes equal to ERANGE when underflow or overflow occurs and for
checking underflow or overflow the strtol returns LONG_MIN and LONG_MAX for
this i had to add climits header file and also i had to add errno.h header
file for errno variable.

Main changes in date.cc are done from line number 151 to 181.
Main changes in datematchdecider.cc are done from line number 89 to 119.

Thank You,
Priyank Bhatt

On 19 December 2014 at 06:14, Olly Betts <olly at survex.com> wrote:
> On Thu, Dec 18, 2014 at 11:38:42PM +0000, James Aylett wrote:
> > Collapsing is done using a valueno (you can tell because collapse_key
> > is of type Xapian::valueno), which is a 32 bit unsigned integer (see
> > here:
> >
>
https://getting-started-with-xapian.readthedocs.org/en/latest/concepts/indexing/values.html
> ).
> > For 32 bit unsigned you need unsigned long.
>
> Xapian::valueno is actually a typedef for unsigned currently.  While the
> standards permit 16 bit integers, no relevant platforms actually use
> them nowadays, so pragmatically unsigned is a better choice if you need
> to pick a single built-in type.
>
> It'd be nice to be a bit smarter about this - now we've decided to
> require C++11 for trunk, we can now use uint32_t here.  Prior to C++11
> I think we'd have to generate the API header based on configure checks
> and then install it to an architecture-dependent path.
>
> But anyway, there's no "strtou()" so you have to converting
to unsigned
> long with strtoul() and then check for potential overflow from the cast
> to Xapian::valueno.
>
> > I think you?ve been looking at other uses of atoi/atol; if you?ve
> > successfully updated code then you should make a pull request or email
> > a patch (or attach one to the trac ticket) so someone can review it
> > and get it into trunk (and hence into a future release). Smaller PRs
> > are generally easier to review than ones that touch lots of files, and
> > it?s not a problem to have a number of small PRs, each fixing atoi()
> > usage in one subsystem of Xapian.
>
> Seconded.
>
> Another reason to get some of your changes reviewed early is in case
> there's something you're doing wrong - better to discover that
before
> you repeat it in a lot of different places.
>
> Cheers,
>     Olly
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.xapian.org/pipermail/xapian-devel/attachments/20141220/5061279c/attachment-0002.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: date.cc
Type: text/x-c++src
Size: 6105 bytes
Desc: not available
URL:
<http://lists.xapian.org/pipermail/xapian-devel/attachments/20141220/5061279c/attachment-0004.cc>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: datematchdecider.cc
Type: text/x-c++src
Size: 4411 bytes
Desc: not available
URL:
<http://lists.xapian.org/pipermail/xapian-devel/attachments/20141220/5061279c/attachment-0005.cc>

On Sat, Dec 20, 2014 at 12:42:08AM +0530, Priyank Bhatt
wrote:
> As James said I am attaching two file *date.cc* and *datematchdecider.cc*
Please send patches, not complete source files:

http://trac.xapian.org/browser/git/xapian-core/HACKING#L1176

Patches are much smaller, and don't require the reviewer to work out the
exact version of the source file you started from.

Cheers,
    Olly