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

replaced getline with fscanf #1136 #1137

Closed
wants to merge 10 commits into from
Closed

Conversation

JayHuLBL
Copy link
Contributor

The alternative of getline() function include function fscanf() and fgets(). However, both of them requires to specify the object with length, to store the result:

int fscanf ( FILE * stream, const char * format, ... ); ...
The additional arguments should point to already allocated objects ...
char * fgets ( char * str, int num, FILE * stream );

fscanf() is used here since it is relatively safe to specify the maximum length of the first section (before first empty space) of each line.

@JayHuLBL JayHuLBL requested a review from mwetter April 30, 2019 04:54
@JayHuLBL JayHuLBL self-assigned this Apr 30, 2019
@mwetter
Copy link
Contributor

mwetter commented Apr 30, 2019

@JayHuLBL This is not save against buffer overflow. Look into using fgets and reallocate memory as needed.

@JayHuLBL
Copy link
Contributor Author

@mwetter It's ready for review.

@JayHuLBL JayHuLBL requested a review from Mathadon April 30, 2019 19:48
@JayHuLBL
Copy link
Contributor Author

@mwetter It has been updated to avoid memory leakage.

The model IBPSA.BoundaryConditions.WeatherData.BaseClasses.Examples.GetTimeSpanTMY3 can run with JModelica without problem.

@mwetter
Copy link
Contributor

mwetter commented May 1, 2019

@JayHuLBL Please also make sure that the failing tests work again.

@JayHuLBL
Copy link
Contributor Author

JayHuLBL commented May 2, 2019

Close this PR as it will be addressed in #1138.

@JayHuLBL JayHuLBL closed this May 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants