Skip to content

Commit

Permalink
make implicit cstring conversions explicit (#19488)
Browse files Browse the repository at this point in the history
The Nim manual says that an implicit conversion to cstring will
eventually not be allowed [1]:

    A Nim `string` is implicitly convertible to `cstring` for convenience.

    [...]

    Even though the conversion is implicit, it is not *safe*: The garbage collector
    does not consider a `cstring` to be a root and may collect the underlying
    memory. For this reason, the implicit conversion will be removed in future
    releases of the Nim compiler. Certain idioms like conversion of a `const` string
    to `cstring` are safe and will remain to be allowed.

And from Nim 1.6.0, such a conversion triggers a warning [2]:

    A dangerous implicit conversion to `cstring` now triggers a `[CStringConv]` warning.
    This warning will become an error in future versions! Use an explicit conversion
    like `cstring(x)` in order to silence the warning.

However, some files in this repo produced such a warning. For example,
before this commit, compiling `parsejson.nim` would produce:

    /foo/Nim/lib/pure/parsejson.nim(221, 37) Warning: implicit conversion to 'cstring' from a non-const location: my.buf; this will become a compile time error in the future [CStringConv]
    /foo/Nim/lib/pure/parsejson.nim(231, 39) Warning: implicit conversion to 'cstring' from a non-const location: my.buf; this will become a compile time error in the future [CStringConv]

This commit resolves the most visible `CStringConv` warnings, making the
cstring conversions explicit.

[1] https://github.com/nim-lang/Nim/blob/d2318d9ccfe6/doc/manual.md#cstring-type
[2] https://github.com/nim-lang/Nim/blob/d2318d9ccfe6/changelogs/changelog_1_6_0.md#type-system
  • Loading branch information
ee7 authored and ringabout committed Aug 19, 2022
1 parent ef92d6e commit 95b4524
Showing 1 changed file with 12 additions and 12 deletions.
24 changes: 12 additions & 12 deletions src/db_postgres.nim
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ proc dbFormat(formatstr: SqlQuery, args: varargs[string]): string =
proc tryExec*(db: DbConn, query: SqlQuery,
args: varargs[string, `$`]): bool {.tags: [ReadDbEffect, WriteDbEffect].} =
## tries to execute the query and returns true if successful, false otherwise.
var res = pqexecParams(db, dbFormat(query, args), 0, nil, nil,
var res = pqexecParams(db, dbFormat(query, args).cstring, 0, nil, nil,
nil, nil, 0)
result = pqresultStatus(res) == PGRES_COMMAND_OK
pqclear(res)
Expand All @@ -143,7 +143,7 @@ proc tryExec*(db: DbConn, stmtName: SqlPrepared,
ReadDbEffect, WriteDbEffect].} =
## tries to execute the query and returns true if successful, false otherwise.
var arr = allocCStringArray(args)
var res = pqexecPrepared(db, stmtName.string, int32(args.len), arr,
var res = pqexecPrepared(db, stmtName.cstring, int32(args.len), arr,
nil, nil, 0)
deallocCStringArray(arr)
result = pqresultStatus(res) == PGRES_COMMAND_OK
Expand All @@ -152,15 +152,15 @@ proc tryExec*(db: DbConn, stmtName: SqlPrepared,
proc exec*(db: DbConn, query: SqlQuery, args: varargs[string, `$`]) {.
tags: [ReadDbEffect, WriteDbEffect].} =
## executes the query and raises EDB if not successful.
var res = pqexecParams(db, dbFormat(query, args), 0, nil, nil,
var res = pqexecParams(db, dbFormat(query, args).cstring, 0, nil, nil,
nil, nil, 0)
if pqresultStatus(res) != PGRES_COMMAND_OK: dbError(db)
pqclear(res)

proc exec*(db: DbConn, stmtName: SqlPrepared,
args: varargs[string]) {.tags: [ReadDbEffect, WriteDbEffect].} =
var arr = allocCStringArray(args)
var res = pqexecPrepared(db, stmtName.string, int32(args.len), arr,
var res = pqexecPrepared(db, stmtName.cstring, int32(args.len), arr,
nil, nil, 0)
deallocCStringArray(arr)
if pqResultStatus(res) != PGRES_COMMAND_OK: dbError(db)
Expand All @@ -172,28 +172,28 @@ proc newRow(L: int): Row =

proc setupQuery(db: DbConn, query: SqlQuery,
args: varargs[string]): PPGresult =
result = pqexec(db, dbFormat(query, args))
result = pqexec(db, dbFormat(query, args).cstring)
if pqResultStatus(result) != PGRES_TUPLES_OK: dbError(db)

proc setupQuery(db: DbConn, stmtName: SqlPrepared,
args: varargs[string]): PPGresult =
var arr = allocCStringArray(args)
result = pqexecPrepared(db, stmtName.string, int32(args.len), arr,
result = pqexecPrepared(db, stmtName.cstring, int32(args.len), arr,
nil, nil, 0)
deallocCStringArray(arr)
if pqResultStatus(result) != PGRES_TUPLES_OK: dbError(db)

proc setupSingeRowQuery(db: DbConn, query: SqlQuery,
args: varargs[string]) =
if pqsendquery(db, dbFormat(query, args)) != 1:
if pqsendquery(db, dbFormat(query, args).cstring) != 1:
dbError(db)
if pqSetSingleRowMode(db) != 1:
dbError(db)

proc setupSingeRowQuery(db: DbConn, stmtName: SqlPrepared,
args: varargs[string]) =
var arr = allocCStringArray(args)
if pqsendqueryprepared(db, stmtName.string, int32(args.len), arr, nil, nil, 0) != 1:
if pqsendqueryprepared(db, stmtName.cstring, int32(args.len), arr, nil, nil, 0) != 1:
dbError(db)
if pqSetSingleRowMode(db) != 1:
dbError(db)
Expand All @@ -205,7 +205,7 @@ proc prepare*(db: DbConn; stmtName: string, query: SqlQuery;
## via `$1`, `$2`, `$3`, etc.
if nParams > 0 and not string(query).contains("$1"):
dbError("parameter substitution expects \"$1\"")
var res = pqprepare(db, stmtName, query.string, int32(nParams), nil)
var res = pqprepare(db, stmtName, query.cstring, int32(nParams), nil)
if pqResultStatus(res) != PGRES_COMMAND_OK: dbError(db)
return SqlPrepared(stmtName)

Expand Down Expand Up @@ -590,7 +590,7 @@ proc execAffectedRows*(db: DbConn, query: SqlQuery,
## executes the query (typically "UPDATE") and returns the
## number of affected rows.
var q = dbFormat(query, args)
var res = pqExec(db, q)
var res = pqExec(db, q.cstring)
if pqresultStatus(res) != PGRES_COMMAND_OK: dbError(db)
result = parseBiggestInt($pqcmdTuples(res))
pqclear(res)
Expand All @@ -601,7 +601,7 @@ proc execAffectedRows*(db: DbConn, stmtName: SqlPrepared,
## executes the query (typically "UPDATE") and returns the
## number of affected rows.
var arr = allocCStringArray(args)
var res = pqexecPrepared(db, stmtName.string, int32(args.len), arr,
var res = pqexecPrepared(db, stmtName.cstring, int32(args.len), arr,
nil, nil, 0)
deallocCStringArray(arr)
if pqresultStatus(res) != PGRES_COMMAND_OK: dbError(db)
Expand Down Expand Up @@ -634,7 +634,7 @@ proc open*(connection, user, password, database: string): DbConn {.
else: substr(connection, 0, colonPos-1)
port = if colonPos < 0: ""
else: substr(connection, colonPos+1)
result = pqsetdbLogin(host, port, nil, nil, database, user, password)
result = pqsetdbLogin(host.cstring, port.cstring, nil, nil, database, user, password)
if pqStatus(result) != CONNECTION_OK: dbError(result) # result = nil

proc setEncoding*(connection: DbConn, encoding: string): bool {.
Expand Down

0 comments on commit 95b4524

Please sign in to comment.