[SERVER-9659] mongo-core/util: Remove atoll and use parseNumberFromString Created: 13/May/13  Updated: 11/Jul/16  Resolved: 10/Jun/13

Status: Closed
Project: Core Server
Component/s: Admin, Internal Code
Affects Version/s: 2.4.3
Fix Version/s: 2.5.1

Type: Improvement Priority: Minor - P4
Reporter: Abhijit Chandrakant Pawar Assignee: Tad Marshall
Resolution: Done Votes: 0
Labels: pull-request
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

Linux 64 bit


Backwards Compatibility: Fully Compatible
Participants:

 Description   

Remove deprecated atoll and use parseNumberFromString while returning the system memory information on linux based machines.



 Comments   
Comment by Tad Marshall [ 11/Jun/13 ]

Manually added comment - Jun 11 2013 13:03:00 PM EST
Author:

{u'date': u'2013-06-10T01:37:27', u'email': u'apawar@cybage.com', u'name': u'Abhijit Pawar'}

Message: core/util: remove atoll and use parsenumberfromstring
Branch: master
https://github.com/mongodb/mongo/commit/283623c7a30326a7baf93fc08ac74162f8d3a50f

Comment by Tad Marshall [ 11/Jun/13 ]

We also have a standard for commits that address "core server" Jira tickets; the header line of the commit message should include the SERVER-xxxx ticket number (in uppercase so a case-sensitive match will find it) in the first line, usually at the beginning of the line though that is not critical. It would be best if the submitter did this, otherwise the committer should edit the commit comment.

This enables Jira to be automatically updated when a commit touches a Jira ticket. Without this, the ticket has to be manually updated.

The commit message for this pull request was:

    core/util: remove atoll and use parsenumberfromstring
    
    This patch replace atoll with stable parsenumberfromstring().

It would have been better as:

    SERVER-9659 remove atoll and use parsenumberfromstring
    
    This patch replace atoll with stable parsenumberfromstring().

Comment by Abhijit Chandrakant Pawar [ 11/Jun/13 ]

Hi Matt,
Thanks for pointing this out. I will make sure that I will add to the same pull request instead of creating new one. I will also follow the coding guidelines carefully.

Thanks,
Abhijit Pawar

Comment by Matt Kangas [ 10/Jun/13 ]

Hi Abhijit, I have have two comments. One, you have now created three separate pull requests for this issue, two of which you have closed. This makes it hard to follow the discussion that we have had on your code.

Please treat a pull request as a logical unit of work. When you have updates, push new commits to the same pull request branch. I will squash multiple commits before merging to master. Or if you want to rebase and "push -f" one new commit, again, push to the same pull request branch.

Second comment, commit 7acb01de8ac still contains tabs for indentation. As I noted in pull/433, this violates our kernel style and can easily be made visible via a git config setting.

git config --local core.whitespace tab-in-indent,tabwidth=4

Since Tad already said "LGTM" and this has taken so long already for a minor change, I will manually fix the leading whitespace issue in your commit and apply. But I do not want to fix your whitespace issues in the future. Please use the git config setting above.

I see that you have four other pull requests open. Please go check them now and verify that they do not have similar whitespace issues.

Comment by Tad Marshall [ 10/Jun/13 ]

Pull request 440 looks good to me.

Comment by Abhijit Chandrakant Pawar [ 10/Jun/13 ]

made changes : I have opened new pull request #440 for this change and closed #433 which had indentation issues.

Comment by Abhijit Chandrakant Pawar [ 29/May/13 ]

I have opened a new pull request #433 for this change.

Comment by Abhijit Chandrakant Pawar [ 28/May/13 ]

Is this to be done for the current case or for all other occurrences of strto* in core?

Also, are we planning to have another version of parseNumberFromString() which supports the string with number + characters to convert?
for example: 1234xyz would be extracted to 1234 and pointer to xyz will be returned for processing?

Comment by Eliot Horowitz (Inactive) [ 25/May/13 ]

We shouldn't use strtoll, but our own new parseNumberFromString found in mongo/base/parse_number.h

Generated at Thu Feb 08 03:21:05 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.