Fix https://github.com/libprima/prima/issues/193
This includes the fixes proposed by @amontoison in https://github.com/libprima/prima/pull/194 to fix https://github.com/libprima/prima/issues/193
Thank you @amontoison for your approval.
Hi @nbelakovski ,
I hope to hear your opinion before merging, especially on the following lines regarding prima_rc_t.
https://github.com/libprima/prima/blob/fix_issue_193/c/prima.c#L124-L125 (cl complains that PRIMA_RESULT_INITIALIZED is not covered explicitly by a case)
https://github.com/libprima/prima/blob/fix_issue_193/c/prima.c#L152-L153 (message was set to NULL, which was suggested by me, but I now think an explicit message is better, more consistent with status, and also safer in case message is printed)
https://github.com/libprima/prima/blob/fix_issue_193/c/prima.c#L196-L199 (info was prima_rc_t, which would cause a warning of incompatible type when &info is passed to xyz_c)
https://github.com/libprima/prima/blob/fix_issue_193/c/prima.c#L237-L238 (It was return PRIMA_INVALID_INPUT; I prefer not to return in the middle; the revised version seems more consistent)
https://github.com/libprima/prima/blob/fix_issue_193/c/prima.c#L241-L245 (It was prima_get_rc_string(info) and return info)
Thank you.
Hi @amontoison and @nbelakovski ,
In case no other solution is proposed, where do you think we should place prima_internal.h? I am not familiar with C conventions, but I thought include/prima was only for the public header. What do others do? I suppose we are not the first to face this situation.
Thanks.
Zaikun
Hi @amontoison and @nbelakovski ,
In case no other solution is proposed, where do you think we should place
prima_internal.h? I am not familiar with C conventions, but I thoughtinclude/primawas only for the public header. What do others do? I suppose we are not the first to face this situation.Thanks.
Zaikun
@zaikunzhang It can stay in include/prima but you just don't install it in ${prefix} folder after the compilation.
@zaikunzhang It can stay in
include/primabut you just don't install it in${prefix}folder after the compilation.
Thank you @amontoison . This would necessitate changing some CMakeList.txt, right? Could you show me how to do it? I have no basic idea bout CMake. Thanks.
@zaikunzhang
It seems that you don't need to do something.
Only prima.h is installed:
https://github.com/libprima/prima/blob/main/c/CMakeLists.txt#L32
Thank you @amontoison for your approval.
Hi @nbelakovski ,
I hope to hear your opinion before merging, especially on the following lines regarding
prima_rc_t.
fix_issue_193/c/prima.c#L124-L125 (clcomplains thatPRIMA_RESULT_INITIALIZEDis not covered explicitly by a case)
fix_issue_193/c/prima.c#L152-L153 (messagewas set toNULL, which was suggested by me, but I now think an explicit message is better, more consistent withstatus, and also safer in casemessageis printed)
fix_issue_193/c/prima.c#L196-L199 (infowasprima_rc_t, which would cause a warning of incompatible type when&infois passed toxyz_c)
fix_issue_193/c/prima.c#L237-L238 (It wasreturn PRIMA_INVALID_INPUT; I prefer not to return in the middle; the revised version seems more consistent)
fix_issue_193/c/prima.c#L241-L245 (It wasprima_get_rc_string(info)andreturn info)Thank you.
Hi @nbelakovski , do these changes look good to you? Thank you.