-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Installation improvements #723
Conversation
77fab4c
to
92906b0
Compare
f776e0e
to
72c628c
Compare
a0eb469
to
40ba5cc
Compare
1bb36da
to
fda12d8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found some plain-old-C stuff that should be easy fixing and not copying data on lioad
libopenage/util/csv.h
Outdated
if (unlikely(strbuf.size() <= line.size())) { | ||
strbuf.resize(line.size() + 1); | ||
} | ||
memcpy(strbuf.data(), line.c_str(), line.size() + 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why memcpy here?
std::copy(line.c_str(), line.c_str()+line.size(), std::back_inserter(strbuf));
libopenage/util/csv.h
Outdated
if (unlikely(strbuf.size() <= line.size())) { | ||
strbuf.resize(line.size() + 1); | ||
} | ||
memcpy(strbuf.data(), line.c_str(), line.size() + 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
memcpy?
libopenage/util/csv.h
Outdated
memcpy(strbuf.data(), line.c_str(), line.size() + 1); | ||
|
||
// use the line copy to fill the current line struct. | ||
int error_column = current_line_data.fill(strbuf.data()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually - Why not give the line.data() directly into the fill() method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because the fill method will modify the data, and i think that line.c_str() is const.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does the fill method modify the data? strbuf is rewritten for every line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was based on strtok
once, not sure if it still does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah now I see what you mean... It is finally consumed here - and there write access is required - which is still not completely broken - if the string is not modified in length one can use &line[0]
- and continue to avoid any copy
libopenage/util/csv.h
Outdated
memcpy(strbuf.data(), line.c_str(), line.size() + 1); | ||
|
||
// use the line copy to fill the current line struct. | ||
int error_column = current_line_data.fill(strbuf.data()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here - use line.data() or
I'll clean the whole csv reading up, it's really a mess... |
Hello everyone, I am liking this project but... why use docx instead of a more documented and libre option as .odt? thanks in advance. |
If I remember correctly the .docx files are in fact csv files. |
@jonaspm, in other words: it's not that docx. |
This file suffix is purely a troll, in fact the files are just plain text. We wanted to use them as as "temporary solution" until we have nyan, but this turned out to be way more complicated, so we still have them as of today, but they will go away at least after my master thesis about nyan. 😛 |
this preserves the union but may slow things down a bit
pythontex.py
(fixes blacklist pythontex in the buildsystem #708)devmode
/usr/share
,/home
,--assets
, ...)std::istream
,std::getline
etcutil::Path
all over the codebase.call()
method for python objectsworks towards #691