prima
prima copied to clipboard
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/prima
was 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/prima
but 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 (cl
complains thatPRIMA_RESULT_INITIALIZED
is not covered explicitly by a case)
fix_issue_193
/c/prima.c#L152-L153 (message
was set toNULL
, which was suggested by me, but I now think an explicit message is better, more consistent withstatus
, and also safer in casemessage
is printed)
fix_issue_193
/c/prima.c#L196-L199 (info
wasprima_rc_t
, which would cause a warning of incompatible type when&info
is 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.