-
Notifications
You must be signed in to change notification settings - Fork 223
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
Modifications to get DocumentServer@core compiling and working under FreeBSD #297
base: develop
Are you sure you want to change the base?
Changes from 8 commits
c668839
521570b
2ea350d
31e4d7f
ff4cef4
5f57938
3219051
f584744
8d8dd40
0580d0a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -77,13 +77,13 @@ namespace DocFileFormat | |
} | ||
|
||
StringTable( VirtualStreamReader *reader, int code_page_ ): | ||
code_page(code_page_), fExtend(false), cbData(0), cbExtra(0), DataExtra(NULL) | ||
code_page(code_page_), fExtend(false), cbData(0), cbExtra(0) | ||
{ | ||
parse( reader, (unsigned int)reader->GetPosition(), 0, false ); | ||
} | ||
|
||
StringTable( POLE::Stream* tableStream, unsigned int fc, unsigned int lcb, int nWordVersion, bool bReadExta = false) : | ||
code_page(1250), fExtend(false), cbData(0), cbExtra(0), DataExtra(NULL) | ||
code_page(1250), fExtend(false), cbData(0), cbExtra(0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DataExtra(0) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand the usefulness of this line since DataExtra is a std::vector which is well initialized without any specific constructor call. |
||
{ | ||
if ( lcb > 0 ) | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,11 @@ char* gcvt(double x, int n, char* b) | |
#define _gcvt gcvt | ||
#endif | ||
|
||
#ifdef __FreeBSD__ | ||
#define _gcvt(x,n,b) sprintf(b, "%.17g", x) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Android uses:
see XtConverter/build/Android/libx2t/src/main/cpp/workaround/gcvt/gcvt.c There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's nearly the same result.. |
||
#endif | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. whitespace There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what do you mean? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
extra blank line There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there's more than a blank line.. I hope it's not the most important thing of this patch.. |
||
|
||
#define DBL_MAX 15 | ||
#define DBL_MAXDIG10 17 | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,10 +39,14 @@ | |
#include <windows.h> | ||
#endif | ||
|
||
#if defined(__linux__) || defined(_MAC) && !defined(_IOS) | ||
#if defined(__linux__) || defined(__FreeBSD__) || defined(_MAC) && !defined(_IOS) | ||
#include <unistd.h> | ||
#include <string.h> | ||
#include <sys/stat.h> | ||
#if defined(__FreeBSD__) | ||
#include <sys/types.h> | ||
#include <sys/sysctl.h> | ||
#endif | ||
#endif | ||
|
||
#ifdef _IOS | ||
|
@@ -1522,6 +1526,22 @@ namespace NSFile | |
return sRet; | ||
#endif | ||
|
||
#if defined(__FreeBSD__) | ||
int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PATHNAME, -1}; | ||
char buf[NS_FILE_MAX_PATH]; | ||
size_t size = NS_FILE_MAX_PATH; | ||
|
||
memset(buf, 0, NS_FILE_MAX_PATH); | ||
if (sysctl(mib, 4, &buf, &size, NULL, 0) != 0) { | ||
size = readlink("/proc/curproc/file", buf, size - 1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. /proc is not mounted by default, not sure it's worth keeping it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can have /proc mounted. sysctl might be enough for most of the cases but if not (I don't know in which case it wouldn't work..) you could still mount /proc and keep that code working There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As you want, I never saw a sysctl failing and if it's the case you're probably doomed somehow. |
||
if (size < 0) | ||
return L""; | ||
} | ||
std::string sUTF8(buf); | ||
std::wstring sRet = CUtf8Converter::GetUnicodeStringFromUTF8((BYTE*)sUTF8.c_str(), sUTF8.length()); | ||
return sRet; | ||
#endif | ||
|
||
return L""; | ||
} | ||
|
||
|
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.
DataExtra(0)
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.
I don't understand the usefulness of this line since DataExtra is a std::vector which is well initialized without any specific constructor call.
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.
I asked dev team and we think better variant is to define
#define NULL 0
in#if
(not sure if translated that right, not good in C Programming)
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.
I would be very interested in having a good explaination about: what is the aim of this line and why using NULL rather than an integer?
Following the vector documentation constructors are:
Since NULL might be assimilated by some compilers to 0 it seems this line is equivalent to
DataExtra(0)
.First, using DataExtra(0) or DataExtra() is equivalent following the documentation (default constructor:
Constructs an empty container, with no elements.
).Next, in C NULL refers to a pointer. That's why I don't understand why using it in that precise case?
Or maybe I missed something but from my point of view this explicit constructor call is just useless (and syntaxically wrong).
Last but not least redefining NULL to 0 might not be a good idea since cast checks would be wrong
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.
I asked our dev @ElenaSubbotina about this part and this may be some leftover from early days
We need some time to figure out if this code really needed in that form and could changes break something else, but currently we have not enought resource, since we are planning to release next major update soon and all work in there
I hope we'll have time to look into it again after major release
Thank you for you job
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.
ok I understand. I hope you'll have time soon. I also hope most of the update (in particular all the preproc conditions and specific code surrounded by FreeBSD preproc conditions) would be added in the next major release.
Maybe you can surround this declaration with #ifdef (and all the code updates you didn't checked yet) in order to have the next major release compiling under FreeBSD (from my point of view, I'm not asking you to try to compile each release under FreeBSD but at least keep patches for that purpose in the master repository).
As you may see some people want/wait/need that feature.
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.
Sorry, I don't think we can do that in next release
I'm sure that in 99% of those statements are correct and will good, but
core
is our base project and any major redone will require a lot of testing from QA and we do not have time for this in our release scheduleThere 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.
even with preproc conditions ?
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.
Sorry, no
New release in final stage of testing and we are not 100% this changes will not cause any side-effects and we miss our deadlines if there will be sideeffects
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.
ok. So I hope it will be added as soon as possible.