Skip to content

Commit

Permalink
Remove potentional nilpointer dereference
Browse files Browse the repository at this point in the history
  • Loading branch information
cabrador committed Dec 7, 2023
1 parent dd12f03 commit 4bc54af
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 6 deletions.
5 changes: 2 additions & 3 deletions executor/extension/tracker/db_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ func makeDbLogger[T any](cfg *utils.Config, log logger.Logger) executor.Extensio
}
}

// PreRun creates a logging file and starts the logging thread.
func (l *dbLogger[T]) PreRun(_ executor.State[T], ctx *executor.Context) error {
// PreRun creates a logging file
func (l *dbLogger[T]) PreRun(executor.State[T], *executor.Context) error {
var err error
l.file, err = os.Create(l.cfg.DbLogging)
if err != nil {
Expand All @@ -56,7 +56,6 @@ func (l *dbLogger[T]) PreRun(_ executor.State[T], ctx *executor.Context) error {
l.wg.Add(1)
go l.doLogging()

ctx.State = proxy.NewLoggerProxy(ctx.State, l.log, l.input)
return nil
}

Expand Down
36 changes: 33 additions & 3 deletions executor/extension/tracker/db_logger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ func TestDbLoggerExtension_LoggingHappens(t *testing.T) {
t.Fatalf("pre-run returned err; %v", err)
}

err = ext.PreTransaction(executor.State[any]{}, ctx)
if err != nil {
t.Fatalf("pre-transaction returned err; %v", err)
}

balance := new(big.Int).SetInt64(10)

beginBlock := fmt.Sprintf("BeginBlock, %v", 1)
Expand Down Expand Up @@ -167,14 +172,16 @@ func TestDbLoggerExtension_PreTransactionDoesNotCreateNewLoggerProxy(t *testing.

ext := MakeDbLogger[any](cfg)

// PreRun assigns LoggerProxy onto ctx.State, hence PreTransaction does no create new
err := ext.PreRun(executor.State[any]{}, ctx)
// first call PreTransaction to assign the proxy
err := ext.PreTransaction(executor.State[any]{}, ctx)
if err != nil {
t.Fatalf("pre-run failed; %v", err)
t.Fatalf("pre-transaction failed; %v", err)
}

// save original state to make sure next call to PreTransaction will not have changed the ctx.State
originalDb := ctx.State

// then make sure it is not re-assigned again
err = ext.PreTransaction(executor.State[any]{}, ctx)
if err != nil {
t.Fatalf("pre-transaction failed; %v", err)
Expand All @@ -184,3 +191,26 @@ func TestDbLoggerExtension_PreTransactionDoesNotCreateNewLoggerProxy(t *testing.
t.Fatal("db must not be be changed!")
}
}

func TestDbLoggerExtension_PreRunDoesNotCreateNewLoggerProxy(t *testing.T) {
ctrl := gomock.NewController(t)
db := state.NewMockStateDB(ctrl)

cfg := &utils.Config{}
cfg.DbLogging = t.TempDir() + "test-log"
cfg.LogLevel = "critical"

ctx := new(executor.Context)
ctx.State = db

ext := MakeDbLogger[any](cfg)

err := ext.PreRun(executor.State[any]{}, ctx)
if err != nil {
t.Fatalf("pre-transaction failed; %v", err)
}

if _, ok := ctx.State.(*proxy.LoggingStateDb); ok {
t.Fatal("db must not be of type LoggingStateDb!")
}
}

0 comments on commit 4bc54af

Please sign in to comment.