Skip to content

Commit c9948e8

Browse files
svigerskesvigerske
committed
do not read parameter current value when creating parameter
- the parameter value may not have been initialized, which raises uninitialized memory read warnings in memory checkers, e.g., valgrind - do no longer call the paramchg callback when creating a parameter, because, in case of a failure, we would try to reset the value to the old value, but now we have no longer read the old value - it also does not seem logical to claim a parameter change at the moment a parameter is created - fixes #68
1 parent 233a610 commit c9948e8

File tree

4 files changed

+55
-35
lines changed

4 files changed

+55
-35
lines changed

‎CHANGELOG‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ Fixed bugs
3838
- respect unboundedness in the computation of activity bounds in conflict.c to avoid invalid huge bounds due to small coefficients on unbounded variables
3939
- ensure positive sides of a linear constraint when recognizing a set partition in rangedRowSimplify() to account for redundancy issues
4040
- relax numerical conditions for variable aggregations to avert invalid variable fixings
41+
- fixed harmless read of uninitialized data when creating parameters
4142

4243
Performance improvements
4344
------------------------
@@ -66,6 +67,11 @@ Build system
6667
- added flag option "sbliss" for SYM variable to specify which graph automorphism package should be used
6768
- use SYM=sbliss by default, since sassy and bliss are now shipped with SCIP
6869

70+
Miscellaneous
71+
-------------
72+
73+
- the parameter change callback is no longer called at the moment a parameter is created
74+
6975
@section RN804 SCIP 8.0.4
7076
*************************
7177

‎src/scip/paramset.c‎

Lines changed: 47 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,7 @@ SCIP_RETCODE paramCopyString(
640640

641641
/* get value of source parameter and copy it to target parameter */
642642
value = SCIPparamGetString(sourceparam);
643-
SCIP_CALL( SCIPparamSetString(targetparam, set, messagehdlr, value, TRUE) );
643+
SCIP_CALL( SCIPparamSetString(targetparam, set, messagehdlr, value, FALSE, TRUE) );
644644

645645
return SCIP_OKAY;
646646
}
@@ -1186,7 +1186,7 @@ SCIP_RETCODE paramCreateString(
11861186
SCIP_ALLOC( BMSduplicateMemoryArray(&(*param)->data.stringparam.defaultvalue, defaultvalue, strlen(defaultvalue)+1) );
11871187
(*param)->data.stringparam.curvalue = NULL;
11881188

1189-
SCIP_CALL( SCIPparamSetString(*param, NULL, messagehdlr, defaultvalue, TRUE) );
1189+
SCIP_CALL( SCIPparamSetString(*param, NULL, messagehdlr, defaultvalue, TRUE, TRUE) );
11901190

11911191
return SCIP_OKAY;
11921192
}
@@ -1412,7 +1412,7 @@ SCIP_RETCODE paramParseString(
14121412
/* remove the quotes */
14131413
valuestr[len-1] = '\0';
14141414
valuestr++;
1415-
SCIP_CALL( SCIPparamSetString(param, set, messagehdlr, valuestr, TRUE) );
1415+
SCIP_CALL( SCIPparamSetString(param, set, messagehdlr, valuestr, FALSE, TRUE) );
14161416

14171417
return SCIP_OKAY;
14181418
}
@@ -2135,7 +2135,7 @@ SCIP_RETCODE SCIPparamsetSetString(
21352135
}
21362136

21372137
/* set the parameter's current value */
2138-
SCIP_CALL( SCIPparamSetString(param, set, messagehdlr, value, TRUE) );
2138+
SCIP_CALL( SCIPparamSetString(param, set, messagehdlr, value, FALSE, TRUE) );
21392139

21402140
return SCIP_OKAY;
21412141
}
@@ -4529,15 +4529,17 @@ SCIP_RETCODE SCIPparamSetBool(
45294529
/* check if the parameter is not fixed */
45304530
SCIP_CALL_QUIET( paramTestFixed(param, messagehdlr) );
45314531

4532+
if( !initialize )
4533+
oldvalue = SCIPparamGetBool(param);
4534+
45324535
/* set the parameter's current value */
4533-
oldvalue = SCIPparamGetBool(param);
45344536
if( param->data.boolparam.valueptr != NULL )
45354537
*param->data.boolparam.valueptr = value;
45364538
else
45374539
param->data.boolparam.curvalue = value;
45384540

4539-
/* call the parameter's change information method */
4540-
if( param->paramchgd != NULL && set != NULL )
4541+
/* call the parameter's change information method, unless initializing */
4542+
if( !initialize && param->paramchgd != NULL && set != NULL )
45414543
{
45424544
SCIP_RETCODE retcode;
45434545

@@ -4546,9 +4548,9 @@ SCIP_RETCODE SCIPparamSetBool(
45464548
if( retcode == SCIP_PARAMETERWRONGVAL )
45474549
{
45484550
if( param->data.boolparam.valueptr != NULL )
4549-
*param->data.boolparam.valueptr = oldvalue;
4551+
*param->data.boolparam.valueptr = oldvalue; /*lint !e644*/
45504552
else
4551-
param->data.boolparam.curvalue = oldvalue;
4553+
param->data.boolparam.curvalue = oldvalue; /*lint !e644*/
45524554
}
45534555
else
45544556
{
@@ -4589,15 +4591,17 @@ SCIP_RETCODE SCIPparamSetInt(
45894591
/* check if the parameter is not fixed */
45904592
SCIP_CALL_QUIET( paramTestFixed(param, messagehdlr) );
45914593

4594+
if( !initialize )
4595+
oldvalue = SCIPparamGetInt(param);
4596+
45924597
/* set the parameter's current value */
4593-
oldvalue = SCIPparamGetInt(param);
45944598
if( param->data.intparam.valueptr != NULL )
45954599
*param->data.intparam.valueptr = value;
45964600
else
45974601
param->data.intparam.curvalue = value;
45984602

4599-
/* call the parameter's change information method */
4600-
if( param->paramchgd != NULL && set != NULL )
4603+
/* call the parameter's change information method, unless initialization */
4604+
if( !initialize && param->paramchgd != NULL && set != NULL )
46014605
{
46024606
SCIP_RETCODE retcode;
46034607

@@ -4606,9 +4610,9 @@ SCIP_RETCODE SCIPparamSetInt(
46064610
if( retcode == SCIP_PARAMETERWRONGVAL )
46074611
{
46084612
if( param->data.intparam.valueptr != NULL )
4609-
*param->data.intparam.valueptr = oldvalue;
4613+
*param->data.intparam.valueptr = oldvalue; /*lint !e644*/
46104614
else
4611-
param->data.intparam.curvalue = oldvalue;
4615+
param->data.intparam.curvalue = oldvalue; /*lint !e644*/
46124616
}
46134617
else
46144618
{
@@ -4649,15 +4653,17 @@ SCIP_RETCODE SCIPparamSetLongint(
46494653
/* check if the parameter is not fixed */
46504654
SCIP_CALL_QUIET( paramTestFixed(param, messagehdlr) );
46514655

4656+
if( !initialize )
4657+
oldvalue = SCIPparamGetLongint(param);
4658+
46524659
/* set the parameter's current value */
4653-
oldvalue = SCIPparamGetLongint(param);
46544660
if( param->data.longintparam.valueptr != NULL )
46554661
*param->data.longintparam.valueptr = value;
46564662
else
46574663
param->data.longintparam.curvalue = value;
46584664

4659-
/* call the parameter's change information method */
4660-
if( param->paramchgd != NULL && set != NULL )
4665+
/* call the parameter's change information method, unless initialization */
4666+
if( !initialize && param->paramchgd != NULL && set != NULL )
46614667
{
46624668
SCIP_RETCODE retcode;
46634669

@@ -4666,9 +4672,9 @@ SCIP_RETCODE SCIPparamSetLongint(
46664672
if( retcode == SCIP_PARAMETERWRONGVAL )
46674673
{
46684674
if( param->data.longintparam.valueptr != NULL )
4669-
*param->data.longintparam.valueptr = oldvalue;
4675+
*param->data.longintparam.valueptr = oldvalue; /*lint !e644*/
46704676
else
4671-
param->data.longintparam.curvalue = oldvalue;
4677+
param->data.longintparam.curvalue = oldvalue; /*lint !e644*/
46724678
}
46734679
else
46744680
{
@@ -4711,15 +4717,17 @@ SCIP_RETCODE SCIPparamSetReal(
47114717
/* check if the parameter is not fixed */
47124718
SCIP_CALL_QUIET( paramTestFixed(param, messagehdlr) );
47134719

4720+
if( !initialize )
4721+
oldvalue = SCIPparamGetReal(param);
4722+
47144723
/* set the parameter's current value */
4715-
oldvalue = SCIPparamGetReal(param);
47164724
if( param->data.realparam.valueptr != NULL )
47174725
*param->data.realparam.valueptr = value;
47184726
else
47194727
param->data.realparam.curvalue = value;
47204728

4721-
/* call the parameter's change information method */
4722-
if( param->paramchgd != NULL && set != NULL )
4729+
/* call the parameter's change information method, unless initializing */
4730+
if( !initialize && param->paramchgd != NULL && set != NULL )
47234731
{
47244732
SCIP_RETCODE retcode;
47254733

@@ -4728,9 +4736,9 @@ SCIP_RETCODE SCIPparamSetReal(
47284736
if( retcode == SCIP_PARAMETERWRONGVAL )
47294737
{
47304738
if( param->data.realparam.valueptr != NULL )
4731-
*param->data.realparam.valueptr = oldvalue;
4739+
*param->data.realparam.valueptr = oldvalue; /*lint !e644*/
47324740
else
4733-
param->data.realparam.curvalue = oldvalue;
4741+
param->data.realparam.curvalue = oldvalue; /*lint !e644*/
47344742
}
47354743
else
47364744
{
@@ -4770,15 +4778,17 @@ SCIP_RETCODE SCIPparamSetChar(
47704778

47714779
SCIP_CALL_QUIET( paramTestFixed(param, messagehdlr) );
47724780

4781+
if( !initialize )
4782+
oldvalue = SCIPparamGetChar(param);
4783+
47734784
/* set the parameter's current value */
4774-
oldvalue = SCIPparamGetChar(param);
47754785
if( param->data.charparam.valueptr != NULL )
47764786
*param->data.charparam.valueptr = value;
47774787
else
47784788
param->data.charparam.curvalue = value;
47794789

4780-
/* call the parameter's change information method */
4781-
if( param->paramchgd != NULL && set != NULL )
4790+
/* call the parameter's change information method, unless initializing */
4791+
if( !initialize && param->paramchgd != NULL && set != NULL )
47824792
{
47834793
SCIP_RETCODE retcode;
47844794

@@ -4787,9 +4797,9 @@ SCIP_RETCODE SCIPparamSetChar(
47874797
if( retcode == SCIP_PARAMETERWRONGVAL )
47884798
{
47894799
if( param->data.charparam.valueptr != NULL )
4790-
*param->data.charparam.valueptr = oldvalue;
4800+
*param->data.charparam.valueptr = oldvalue; /*lint !e644*/
47914801
else
4792-
param->data.charparam.curvalue = oldvalue;
4802+
param->data.charparam.curvalue = oldvalue; /*lint !e644*/
47934803
}
47944804
else
47954805
{
@@ -4812,6 +4822,7 @@ SCIP_RETCODE SCIPparamSetString(
48124822
SCIP_SET* set, /**< global SCIP settings, or NULL if param change method should not be called */
48134823
SCIP_MESSAGEHDLR* messagehdlr, /**< message handler */
48144824
const char* value, /**< new value of the parameter */
4825+
SCIP_Bool initialize, /**< is this the initialization of the parameter? */
48154826
SCIP_Bool quiet /**< should the parameter be set quiet (no output) */
48164827
)
48174828
{
@@ -4826,17 +4837,19 @@ SCIP_RETCODE SCIPparamSetString(
48264837
/* set the parameter's current value */
48274838
if( param->data.stringparam.valueptr != NULL )
48284839
{
4829-
oldvalue = *param->data.stringparam.valueptr;
4840+
if( !initialize )
4841+
oldvalue = *param->data.stringparam.valueptr;
48304842
SCIP_ALLOC( BMSduplicateMemoryArray(param->data.stringparam.valueptr, value, strlen(value)+1) );
48314843
}
48324844
else
48334845
{
4834-
oldvalue = param->data.stringparam.curvalue;
4846+
if( !initialize )
4847+
oldvalue = param->data.stringparam.curvalue;
48354848
SCIP_ALLOC( BMSduplicateMemoryArray(&param->data.stringparam.curvalue, value, strlen(value)+1) );
48364849
}
48374850

4838-
/* call the parameter's change information method */
4839-
if( param->paramchgd != NULL && set != NULL )
4851+
/* call the parameter's change information method, unless initializing */
4852+
if( !initialize && param->paramchgd != NULL && set != NULL )
48404853
{
48414854
SCIP_RETCODE retcode;
48424855

@@ -4993,7 +5006,7 @@ SCIP_RETCODE SCIPparamSetToDefault(
49935006
break;
49945007

49955008
case SCIP_PARAMTYPE_STRING:
4996-
SCIP_CALL( SCIPparamSetString(param, set, messagehdlr, SCIPparamGetStringDefault(param), TRUE) );
5009+
SCIP_CALL( SCIPparamSetString(param, set, messagehdlr, SCIPparamGetStringDefault(param), FALSE, TRUE) );
49975010
break;
49985011

49995012
default:

‎src/scip/paramset.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,7 @@ SCIP_RETCODE SCIPparamSetString(
523523
SCIP_SET* set, /**< global SCIP settings, or NULL if param change method should not be called */
524524
SCIP_MESSAGEHDLR* messagehdlr, /**< message handler */
525525
const char* value, /**< new value of the parameter */
526+
SCIP_Bool initialize, /**< is this the initialization of the parameter? */
526527
SCIP_Bool quiet /**< should the parameter be set quiet (no output) */
527528
);
528529

‎src/scip/set.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3414,7 +3414,7 @@ SCIP_RETCODE SCIPsetChgStringParam(
34143414
assert(set != NULL);
34153415
assert(param != NULL);
34163416

3417-
retcode = SCIPparamSetString(param, set, messagehdlr, value, TRUE);
3417+
retcode = SCIPparamSetString(param, set, messagehdlr, value, FALSE, TRUE);
34183418

34193419
if( retcode != SCIP_PARAMETERWRONGVAL )
34203420
{

0 commit comments

Comments
 (0)