-
Notifications
You must be signed in to change notification settings - Fork 154
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
Fix GCC 13.2.1 compilation with -Werror #197
base: master
Are you sure you want to change the base?
Conversation
7bbfa88
to
9fc31f3
Compare
@@ -197,6 +197,9 @@ template <class Type> class ElFifo | |||
|
|||
void incr_capa() | |||
{ | |||
if (_capa == 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.
Should be
if (_capa < 1)
_capa = 1;
in constructor?
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 can be, I did this in case _capa=-1 means uninitialized where _capa = 0 is an error
@@ -202,7 +202,7 @@ template <typename T> | |||
node_max[1] = nodes[1] + node_num_max; | |||
if (stage) | |||
{ | |||
memmove(nodes[1], (char*)nodes[0] + ((char*)nodes_old[1] - (char*)nodes_old[0]), node_num*sizeof(Node)); |
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 not declaring size
here?
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.
The value needs to be computed before the realloc to avoid use after free
Je manque un peu de contexte, cela dit je vois pas cette ligne dans les ElFifo
De: "JM Muller" ***@***.***>
À: "micmacIGN/micmac" ***@***.***>
Cc: "Marc Pierrot-Deseilligny" ***@***.***>, "Review requested" ***@***.***>
Envoyé: Jeudi 17 Août 2023 15:01:42
Objet: Re: [micmacIGN/micmac] Fix GCC 12.1.0 compilation with -Werror (PR #197)
@jmmuller commented on this pull request.
In [ #197 (comment) | include/ext_stl/fifo.h ] :
@@ -197,6 +197,9 @@ template <class Type> class ElFifo
void incr_capa()
{
+ if (_capa == 0)
Should be
if (_capa < 1)
_capa = 1;
in constructor?
In [ #197 (comment) | src/uti_phgrm/GraphCut/QPBO-v1.4/QPBO.cpp ] :
@@ -202,7 +202,7 @@ template <typename T>
node_max[1] = nodes[1] + node_num_max;
if (stage)
{
- memmove(nodes[1], (char*)nodes[0] + ((char*)nodes_old[1] - (char*)nodes_old[0]), node_num*sizeof(Node));
Why not declaring size here?
—
Reply to this email directly, [ #197 (review) | view it on GitHub ] , or [ https://github.com/notifications/unsubscribe-auth/AFROVJQIAXQPAVFIGPIDJD3XVYI3NANCNFSM5YNVEMPA | unsubscribe ] .
You are receiving this because your review was requested. Message ID: ***@***.***>
|
Hello !
Charles a fait des corrections parce que son compilateur a un peu plus de warnings que nous.
Il a fait une pull request pour ces modifs :
https://github.com/micmacIGN/micmac/pull/197/files
Dans incr_capa() il a rajouté un test de _capa==0 et mise à 1 pour éviter les accidents. (Je ne sais pas quel message le compilateur lui a dit pour remarquer le problème).
Je propose juste de passer ce test dans le constructeur parce que autant régler le pb tout de suite ?
De: "pierrot deseilligny" ***@***.***>
À: "micmacIGN/micmac" ***@***.***>
Cc: "Jean-Michael Muller" ***@***.***>, "Mention" ***@***.***>
Envoyé: Jeudi 17 Août 2023 15:11:04
Objet: Re: [micmacIGN/micmac] Fix GCC 13.2.1 compilation with -Werror (PR #197)
Je manque un peu de contexte, cela dit je vois pas cette ligne dans les ElFifo
De: "JM Muller" ***@***.***>
À: "micmacIGN/micmac" ***@***.***>
Cc: "Marc Pierrot-Deseilligny" ***@***.***>, "Review requested" ***@***.***>
Envoyé: Jeudi 17 Août 2023 15:01:42
Objet: Re: [micmacIGN/micmac] Fix GCC 12.1.0 compilation with -Werror (PR #197)
@jmmuller commented on this pull request.
In [ #197 (comment) | include/ext_stl/fifo.h ] :
@@ -197,6 +197,9 @@ template <class Type> class ElFifo
void incr_capa()
{
+ if (_capa == 0)
Should be
if (_capa < 1)
_capa = 1;
in constructor?
In [ #197 (comment) | src/uti_phgrm/GraphCut/QPBO-v1.4/QPBO.cpp ] :
@@ -202,7 +202,7 @@ template <typename T>
node_max[1] = nodes[1] + node_num_max;
if (stage)
{
- memmove(nodes[1], (char*)nodes[0] + ((char*)nodes_old[1] - (char*)nodes_old[0]), node_num*sizeof(Node));
Why not declaring size here?
—
Reply to this email directly, [ #197 (review) | view it on GitHub ] , or [ https://github.com/notifications/unsubscribe-auth/AFROVJQIAXQPAVFIGPIDJD3XVYI3NANCNFSM5YNVEMPA | unsubscribe ] .
You are receiving this because your review was requested. Message ID: ***@***.***>
—
Reply to this email directly, [ #197 (comment) | view it on GitHub ] , or [ https://github.com/notifications/unsubscribe-auth/AF3NYM4IRVNTY4K6H76YZMTXVYJ6RANCNFSM5YNVEMPA | unsubscribe ] .
You are receiving this because you were mentioned. Message ID: ***@***.***>
|
Mostly "maybe uninitialized" variable making the compilator not happy and use of pointer arithmetic with freed pointer which trigger compilation warning/error.
Need to check if graph cut reallocation code still used. If yes need to insure that the old freed pointer not used to suppress the compilation error.
Maybe others compiler will trigger other errors.