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

fread: change return types of all field parsers #2234

Merged
merged 4 commits into from
Jun 30, 2017
Merged

fread: change return types of all field parsers #2234

merged 4 commits into from
Jun 30, 2017

Conversation

st-pasha
Copy link
Contributor

Previously all field parsers returned _Bool status, with true indicating a successful parse, and false indicating a failure.

This PR makes it so that all field parsers return an int, with 0 indicating success and 1 indicating failure. This is equivalent to applying logical not to all return values. The reason for such change is that it will allow us to have more than one type of failure in the future (i.e. 1 indicating regular failure, 2 indicating recoverable failure, etc.) These extend return types will be used in PR #2200

Additionally, this PR modifies the type of the target parameter: each field parser will have the target variable suitable for that particular parser. This improves code readability and serves as additional documentation as to the purpose of each parser.

In some places where trash variable were declared as char[8] those were replaced with double or int64_t or similar -- this is to ensure proper alignment of the variable (char[8] may be byte-aligned, whereas double is aligned to 8-byte boundary).

@st-pasha st-pasha added the fread label Jun 30, 2017
@st-pasha st-pasha requested a review from mattdowle June 30, 2017 03:45
@codecov-io
Copy link

codecov-io commented Jun 30, 2017

Codecov Report

Merging #2234 into master will increase coverage by <.01%.
The diff coverage is 98.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2234      +/-   ##
==========================================
+ Coverage   90.71%   90.71%   +<.01%     
==========================================
  Files          59       59              
  Lines       11507    11508       +1     
==========================================
+ Hits        10438    10439       +1     
  Misses       1069     1069
Impacted Files Coverage Δ
src/fread.c 93.02% <98.14%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7d5ec52...5f17cdd. Read the comment docs.

st-pasha and others added 2 commits June 29, 2017 23:30
typedef updated from _Bool to int too.  Reduced trash variable variants.
@mattdowle mattdowle merged commit bfa0816 into Rdatatable:master Jun 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants