Skip to content

Commit

Permalink
Fix integer parsing logic
Browse files Browse the repository at this point in the history
We have various places where the types of what we parse and then use
don't align. This is something CodeQL correctly warns about that it can
lead to unexpected behavior with truncation.

This fixes those cases that CodeQL is complaining about.

Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
  • Loading branch information
dbussink committed Jan 29, 2025
1 parent 9c6c380 commit 7912999
Show file tree
Hide file tree
Showing 12 changed files with 45 additions and 45 deletions.
2 changes: 1 addition & 1 deletion go/vt/vtcombo/tablet_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -1005,7 +1005,7 @@ func (itmc *internalTabletManagerClient) PopulateReparentJournal(context.Context
}

// ReadReparentJournalInfo is part of the tmclient.TabletManagerClient interface.
func (itmc *internalTabletManagerClient) ReadReparentJournalInfo(ctx context.Context, tablet *topodatapb.Tablet) (int, error) {
func (itmc *internalTabletManagerClient) ReadReparentJournalInfo(ctx context.Context, tablet *topodatapb.Tablet) (int32, error) {
return 0, fmt.Errorf("not implemented in vtcombo")
}

Expand Down
4 changes: 2 additions & 2 deletions go/vt/vtctl/grpcvtctldserver/testutil/test_tmclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ type TabletManagerClient struct {
// keyed by tablet alias
PopulateReparentJournalResults map[string]error
// keyed by tablet alias
ReadReparentJournalInfoResults map[string]int
ReadReparentJournalInfoResults map[string]int32
// keyed by tablet alias.
PromoteReplicaDelays map[string]time.Duration
// keyed by tablet alias. injects a sleep to the end of the function
Expand Down Expand Up @@ -974,7 +974,7 @@ func (fake *TabletManagerClient) PopulateReparentJournal(ctx context.Context, ta
}

// ReadReparentJournalInfo is part of the tmclient.TabletManagerClient interface.
func (fake *TabletManagerClient) ReadReparentJournalInfo(ctx context.Context, tablet *topodatapb.Tablet) (int, error) {
func (fake *TabletManagerClient) ReadReparentJournalInfo(ctx context.Context, tablet *topodatapb.Tablet) (int32, error) {
if fake.ReadReparentJournalInfoResults == nil {
return 1, nil
}
Expand Down
8 changes: 4 additions & 4 deletions go/vt/vtctl/reparentutil/emergency_reparenter.go
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ func (erp *EmergencyReparenter) findErrantGTIDs(
}

// Find the maximum length of the reparent journal among all the candidates.
var maxLen int
var maxLen int32
for _, length := range reparentJournalLen {
maxLen = max(maxLen, length)
}
Expand Down Expand Up @@ -902,8 +902,8 @@ func (erp *EmergencyReparenter) gatherReparenJournalInfo(
validCandidates map[string]replication.Position,
tabletMap map[string]*topo.TabletInfo,
waitReplicasTimeout time.Duration,
) (map[string]int, error) {
reparentJournalLen := make(map[string]int)
) (map[string]int32, error) {
reparentJournalLen := make(map[string]int32)
var mu sync.Mutex
errCh := make(chan concurrency.Error)
defer close(errCh)
Expand All @@ -916,7 +916,7 @@ func (erp *EmergencyReparenter) gatherReparenJournalInfo(
for candidate := range validCandidates {
go func(alias string) {
var err error
var length int
var length int32
defer func() {
errCh <- concurrency.Error{
Err: err,
Expand Down
28 changes: 14 additions & 14 deletions go/vt/vtctl/reparentutil/emergency_reparenter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4637,7 +4637,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
{
name: "Case 1a: No Errant GTIDs. This is the first reparent. A replica is the most advanced.",
tmc: &testutil.TabletManagerClient{
ReadReparentJournalInfoResults: map[string]int{
ReadReparentJournalInfoResults: map[string]int32{
"zone1-0000000102": 1,
"zone1-0000000103": 1,
"zone1-0000000104": 1,
Expand Down Expand Up @@ -4733,7 +4733,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
},
},
tmc: &testutil.TabletManagerClient{
ReadReparentJournalInfoResults: map[string]int{
ReadReparentJournalInfoResults: map[string]int32{
"zone1-0000000102": 2,
"zone1-0000000103": 2,
"zone1-0000000104": 2,
Expand Down Expand Up @@ -4797,7 +4797,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
},
},
tmc: &testutil.TabletManagerClient{
ReadReparentJournalInfoResults: map[string]int{
ReadReparentJournalInfoResults: map[string]int32{
"zone1-0000000102": 2,
"zone1-0000000103": 2,
"zone1-0000000104": 2,
Expand Down Expand Up @@ -4861,7 +4861,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
},
},
tmc: &testutil.TabletManagerClient{
ReadReparentJournalInfoResults: map[string]int{
ReadReparentJournalInfoResults: map[string]int32{
"zone1-0000000102": 3,
"zone1-0000000103": 2,
"zone1-0000000104": 1,
Expand Down Expand Up @@ -4925,7 +4925,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
},
},
tmc: &testutil.TabletManagerClient{
ReadReparentJournalInfoResults: map[string]int{
ReadReparentJournalInfoResults: map[string]int32{
"zone1-0000000102": 2,
"zone1-0000000103": 2,
"zone1-0000000104": 1,
Expand Down Expand Up @@ -4989,7 +4989,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
},
},
tmc: &testutil.TabletManagerClient{
ReadReparentJournalInfoResults: map[string]int{
ReadReparentJournalInfoResults: map[string]int32{
"zone1-0000000102": 2,
"zone1-0000000103": 3,
"zone1-0000000104": 3,
Expand Down Expand Up @@ -5052,7 +5052,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
},
},
tmc: &testutil.TabletManagerClient{
ReadReparentJournalInfoResults: map[string]int{
ReadReparentJournalInfoResults: map[string]int32{
"zone1-0000000102": 2,
"zone1-0000000103": 3,
"zone1-0000000104": 2,
Expand Down Expand Up @@ -5116,7 +5116,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
},
},
tmc: &testutil.TabletManagerClient{
ReadReparentJournalInfoResults: map[string]int{
ReadReparentJournalInfoResults: map[string]int32{
"zone1-0000000102": 2,
"zone1-0000000103": 3,
"zone1-0000000104": 2,
Expand Down Expand Up @@ -5179,7 +5179,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
},
},
tmc: &testutil.TabletManagerClient{
ReadReparentJournalInfoResults: map[string]int{
ReadReparentJournalInfoResults: map[string]int32{
"zone1-0000000102": 3,
"zone1-0000000103": 3,
"zone1-0000000104": 3,
Expand Down Expand Up @@ -5243,7 +5243,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
},
},
tmc: &testutil.TabletManagerClient{
ReadReparentJournalInfoResults: map[string]int{
ReadReparentJournalInfoResults: map[string]int32{
"zone1-0000000102": 3,
"zone1-0000000103": 3,
"zone1-0000000104": 2,
Expand Down Expand Up @@ -5297,7 +5297,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
},
},
tmc: &testutil.TabletManagerClient{
ReadReparentJournalInfoResults: map[string]int{
ReadReparentJournalInfoResults: map[string]int32{
"zone1-0000000102": 3,
"zone1-0000000103": 3,
},
Expand Down Expand Up @@ -5354,7 +5354,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
},
},
tmc: &testutil.TabletManagerClient{
ReadReparentJournalInfoResults: map[string]int{
ReadReparentJournalInfoResults: map[string]int32{
"zone1-0000000102": 3,
"zone1-0000000103": 3,
"zone1-0000000104": 3,
Expand Down Expand Up @@ -5418,7 +5418,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
},
},
tmc: &testutil.TabletManagerClient{
ReadReparentJournalInfoResults: map[string]int{
ReadReparentJournalInfoResults: map[string]int32{
"zone1-0000000102": 2,
"zone1-0000000103": 3,
"zone1-0000000104": 2,
Expand Down Expand Up @@ -5481,7 +5481,7 @@ func TestEmergencyReparenterFindErrantGTIDs(t *testing.T) {
},
},
tmc: &testutil.TabletManagerClient{
ReadReparentJournalInfoResults: map[string]int{},
ReadReparentJournalInfoResults: map[string]int32{},
},
statusMap: map[string]*replicationdatapb.StopReplicationStatus{
"zone1-0000000103": {
Expand Down
28 changes: 14 additions & 14 deletions go/vt/vttablet/common/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,18 +142,18 @@ func NewVReplicationConfig(overrides map[string]string) (*VReplicationConfig, er
c.ExperimentalFlags = value
}
case "vreplication_net_read_timeout":
value, err := strconv.ParseInt(v, 10, 64)
value, err := strconv.Atoi(v)
if err != nil {
errors = append(errors, getError(k, v))
} else {
c.NetReadTimeout = int(value)
c.NetReadTimeout = value
}
case "vreplication_net_write_timeout":
value, err := strconv.ParseInt(v, 10, 64)
value, err := strconv.Atoi(v)
if err != nil {
errors = append(errors, getError(k, v))
} else {
c.NetWriteTimeout = int(value)
c.NetWriteTimeout = value
}
case "vreplication_copy_phase_duration":
value, err := time.ParseDuration(v)
Expand All @@ -177,18 +177,18 @@ func NewVReplicationConfig(overrides map[string]string) (*VReplicationConfig, er
c.MaxTimeToRetryError = value
}
case "relay_log_max_size":
value, err := strconv.ParseInt(v, 10, 64)
value, err := strconv.Atoi(v)
if err != nil {
errors = append(errors, getError(k, v))
} else {
c.RelayLogMaxSize = int(value)
c.RelayLogMaxSize = value
}
case "relay_log_max_items":
value, err := strconv.ParseInt(v, 10, 64)
value, err := strconv.Atoi(v)
if err != nil {
errors = append(errors, getError(k, v))
} else {
c.RelayLogMaxItems = int(value)
c.RelayLogMaxItems = value
}
case "vreplication_replica_lag_tolerance":
value, err := time.ParseDuration(v)
Expand All @@ -198,11 +198,11 @@ func NewVReplicationConfig(overrides map[string]string) (*VReplicationConfig, er
c.ReplicaLagTolerance = value
}
case "vreplication_heartbeat_update_interval":
value, err := strconv.ParseInt(v, 10, 64)
value, err := strconv.Atoi(v)
if err != nil {
errors = append(errors, getError(k, v))
} else {
c.HeartbeatUpdateInterval = int(value)
c.HeartbeatUpdateInterval = value
}
case "vreplication_store_compressed_gtid":
value, err := strconv.ParseBool(v)
Expand All @@ -212,19 +212,19 @@ func NewVReplicationConfig(overrides map[string]string) (*VReplicationConfig, er
c.StoreCompressedGTID = value
}
case "vreplication-parallel-insert-workers":
value, err := strconv.ParseInt(v, 10, 64)
value, err := strconv.Atoi(v)
if err != nil {
errors = append(errors, getError(k, v))
} else {
c.ParallelInsertWorkers = int(value)
c.ParallelInsertWorkers = value
}
case "vstream_packet_size":
value, err := strconv.ParseInt(v, 10, 64)
value, err := strconv.Atoi(v)
if err != nil {
errors = append(errors, getError(k, v))
} else {
c.VStreamPacketSizeOverride = true
c.VStreamPacketSize = int(value)
c.VStreamPacketSize = value
}
case "vstream_dynamic_packet_size":
value, err := strconv.ParseBool(v)
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vttablet/faketmclient/fake_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ func (client *FakeTabletManagerClient) PopulateReparentJournal(ctx context.Conte
}

// ReadReparentJournalInfo is part of the tmclient.TabletManagerClient interface.
func (client *FakeTabletManagerClient) ReadReparentJournalInfo(ctx context.Context, tablet *topodatapb.Tablet) (int, error) {
func (client *FakeTabletManagerClient) ReadReparentJournalInfo(ctx context.Context, tablet *topodatapb.Tablet) (int32, error) {
return 10, nil
}

Expand Down
4 changes: 2 additions & 2 deletions go/vt/vttablet/grpctmclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1124,7 +1124,7 @@ func (client *Client) PopulateReparentJournal(ctx context.Context, tablet *topod
}

// ReadReparentJournalInfo is part of the tmclient.TabletManagerClient interface.
func (client *Client) ReadReparentJournalInfo(ctx context.Context, tablet *topodatapb.Tablet) (int, error) {
func (client *Client) ReadReparentJournalInfo(ctx context.Context, tablet *topodatapb.Tablet) (int32, error) {
c, closer, err := client.dialer.dial(ctx, tablet)
if err != nil {
return 0, err
Expand All @@ -1134,7 +1134,7 @@ func (client *Client) ReadReparentJournalInfo(ctx context.Context, tablet *topod
if err != nil {
return 0, err
}
return int(resp.Length), nil
return resp.Length, nil
}

// InitReplica is part of the tmclient.TabletManagerClient interface.
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vttablet/tabletmanager/rpc_agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ type RPCTM interface {

PopulateReparentJournal(ctx context.Context, timeCreatedNS int64, actionName string, tabletAlias *topodatapb.TabletAlias, pos string) error

ReadReparentJournalInfo(ctx context.Context) (int, error)
ReadReparentJournalInfo(ctx context.Context) (int32, error)

InitReplica(ctx context.Context, parent *topodatapb.TabletAlias, replicationPosition string, timeCreatedNS int64, semiSync bool) error

Expand Down
4 changes: 2 additions & 2 deletions go/vt/vttablet/tabletmanager/rpc_replication.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ func (tm *TabletManager) PopulateReparentJournal(ctx context.Context, timeCreate
}

// ReadReparentJournalInfo reads the information from reparent journal.
func (tm *TabletManager) ReadReparentJournalInfo(ctx context.Context) (int, error) {
func (tm *TabletManager) ReadReparentJournalInfo(ctx context.Context) (int32, error) {
log.Infof("ReadReparentJournalInfo")
if err := tm.waitForGrantsToHaveApplied(ctx); err != nil {
return 0, err
Expand All @@ -442,7 +442,7 @@ func (tm *TabletManager) ReadReparentJournalInfo(ctx context.Context) (int, erro
if len(res.Rows) != 1 {
return 0, vterrors.Errorf(vtrpc.Code_INTERNAL, "unexpected rows when reading reparent journal, got %v", len(res.Rows))
}
return res.Rows[0][0].ToInt()
return res.Rows[0][0].ToInt32()
}

// InitReplica sets replication primary and position, and waits for the
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vttablet/tabletserver/twopc.go
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ func (tpc *TwoPC) UnresolvedTransactions(ctx context.Context, abandonTime time.T
appendCurrentTx()

// Extract the transaction state and initialize a new TransactionMetadata
stateID, _ := row[1].ToInt()
stateID, _ := row[1].ToInt32()
timeCreated, _ := row[2].ToCastInt64()
currentTx = &querypb.TransactionMetadata{
Dtid: dtid,
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vttablet/tmclient/rpc_client_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ type TabletManagerClient interface {
PopulateReparentJournal(ctx context.Context, tablet *topodatapb.Tablet, timeCreatedNS int64, actionName string, tabletAlias *topodatapb.TabletAlias, pos string) error

// ReadReparentJournalInfo reads the information from reparent journal
ReadReparentJournalInfo(ctx context.Context, tablet *topodatapb.Tablet) (int, error)
ReadReparentJournalInfo(ctx context.Context, tablet *topodatapb.Tablet) (int32, error)

// InitReplica tells a tablet to start replicating from the
// passed in primary tablet alias, and wait for the row in the
Expand Down
4 changes: 2 additions & 2 deletions go/vt/vttablet/tmrpctest/test_tm_rpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -1179,9 +1179,9 @@ func (fra *fakeRPCTM) PopulateReparentJournal(ctx context.Context, timeCreatedNS
return nil
}

var testReparentJournalLen = 10
var testReparentJournalLen int32 = 10

func (fra *fakeRPCTM) ReadReparentJournalInfo(context.Context) (int, error) {
func (fra *fakeRPCTM) ReadReparentJournalInfo(context.Context) (int32, error) {
if fra.panics {
panic(fmt.Errorf("test-triggered panic"))
}
Expand Down

0 comments on commit 7912999

Please sign in to comment.