Add golangci-lint and fix lint issues; add ci for macOS and windows #36
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dongbeiouba
force-pushed
the
ci/win_mac
branch
30 times, most recently
from
0ce2bfa to
8f448d8
Compare
dongbeiouba
force-pushed
the
ci/win_mac
branch
8 times, most recently
from
85aaaf7 to
348c87a
Compare
itomsawyer
reviewed
Nov 12, 2024
crypto/ciphers.go
Outdated
| "fmt" | ||
| "runtime" | ||
| "unsafe" | ||
| ) | ||
|
|
||
| const ( | ||
| GCM_TAG_MAXLEN = 16 | ||
| GcmTagMaxLen = 16 |
There was a problem hiding this comment.
按照go的命名规格，如果本身是缩写，那就保留大写 GcmTagMaxLen => GCMTagMaxLen
itomsawyer
reviewed
Nov 12, 2024
crypto/sm2/sm2.go
Outdated
| @@ -24,27 +20,37 @@ import ( | |||
|
|
|||
| // VerifyASN1 verifies ASN.1 encoded signature. Returns nil on success. | |||
| func VerifyASN1(pub crypto.PublicKey, data, sig []byte) error { | |||
| if pub.KeyType() != crypto.NID_sm2 { | |||
| return errors.New("SM2: key type is not sm2") | |||
| if pub.KeyType() != crypto.NidSm2 { | |||
itomsawyer
reviewed
Nov 12, 2024
ssl.go
Outdated
| @@ -32,37 +32,35 @@ const ( | |||
| ) | |||
|
|
|||
| const ( | |||
| OPENSSL_NPN_NEGOTIATED C.int = C.OPENSSL_NPN_NEGOTIATED | |||
| OPENSSL_NPN_NO_OVERLAP C.int = C.OPENSSL_NPN_NO_OVERLAP | |||
| NpnNegotiated C.int = C.OPENSSL_NPN_NEGOTIATED | |||
itomsawyer
reviewed
Nov 12, 2024
| //export sni_cb_thunk | ||
| func sni_cb_thunk(p unsafe.Pointer, con *C.SSL, ad unsafe.Pointer, arg unsafe.Pointer) C.int { | ||
| //export sniCbThunk | ||
| func sniCbThunk(callback unsafe.Pointer, con *C.SSL, ad unsafe.Pointer, arg unsafe.Pointer) C.int { |
There was a problem hiding this comment.
func sniCbThunk(callback unsafe.Pointer, con *C.SSL, _ unsafe.Pointer, _ unsafe.Pointer)
itomsawyer
reviewed
Nov 12, 2024
ssl_test.go
Outdated
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| _ = serverConn.Close() |
There was a problem hiding this comment.
Why ignore the error return by Close() ?
itomsawyer
reviewed
Nov 12, 2024
| if err != nil { | ||
| t.Error(err) | ||
| } | ||
| tls_key, ok := tls_cert.PrivateKey.(*rsa.PrivateKey) | ||
|
|
||
| tlsKey, ok := tlsCert.PrivateKey.(*rsa.PrivateKey) |
There was a problem hiding this comment.
_, ok := tlsCert.PrivateKey.(*rsa.PrivateKey)
itomsawyer
reviewed
Nov 12, 2024
| ctx := C.X_EVP_MD_CTX_new() | ||
| defer C.X_EVP_MD_CTX_free(ctx) | ||
|
|
||
| if key.KeyType() == KeyTypeED25519 { | ||
| // do ED specific one-shot sign | ||
| if method != nil || len(data) == 0 { | ||
| return nil, errors.New("signpkcs1v15: 0-length data or non-null digest") | ||
| return nil, ErrNilParameter |
There was a problem hiding this comment.
我觉得没啥必要，ErrNilParameter表示参数为空就可以了
There was a problem hiding this comment.
嗯，这里是keytype 是specific KeyTypeED25519的时候对应的错误，加wrapper 可能更好区分。如果上层不需要区分也OK
itomsawyer
reviewed
Nov 12, 2024
| bio := C.BIO_new_mem_buf(unsafe.Pointer(&pem_block[0]), | ||
| C.int(len(pem_block))) | ||
|
|
||
| bio := C.BIO_new_mem_buf(unsafe.Pointer(&pemBlock[0]), C.int(len(pemBlock))) |
There was a problem hiding this comment.
没必要使用C.CBytes()拷贝一份内存
itomsawyer
reviewed
Nov 13, 2024
conn.go
Outdated
| want_read_future *utils.Future | ||
| conn net.Conn | ||
| ctx *Ctx // for gc | ||
| intoSsl *crypto.ReadBio |
itomsawyer
reviewed
Nov 13, 2024
| return 0, func() error { return io.EOF } | ||
| } | ||
| runtime.LockOSThread() | ||
| defer runtime.UnlockOSThread() | ||
| rv, errno := C.SSL_read(c.ssl, unsafe.Pointer(&b[0]), C.int(len(b))) | ||
| rv, errno := C.SSL_read(c.ssl, unsafe.Pointer(&buf[0]), C.int(len(buf))) |
There was a problem hiding this comment.
SSL_read()直接写到buf里面，不用使用C.CBytes()
There was a problem hiding this comment.
OK， 确保c 不会free go alloc 的内存就没问题
itomsawyer
reviewed
Nov 13, 2024
crypto/cert.go
Outdated
| } | ||
| runtime.LockOSThread() | ||
| defer runtime.UnlockOSThread() | ||
| bio := C.BIO_new_mem_buf(unsafe.Pointer(&pem_block[0]), | ||
| C.int(len(pem_block))) | ||
| bio := C.BIO_new_mem_buf(unsafe.Pointer(&pemBlock[0]), |
There was a problem hiding this comment.
没必要使用C.CBytes()拷贝一份内存
There was a problem hiding this comment.
OK， 确保c 不会free go alloc 的内存就没问题
CI adds golangci-lint check. Delete duplicate cgo ldflags -lcrypto Fix golangci-lint issues. - Set max length of line to 120 - Test cases add t.Parallel() - Test helpers add t.Helper() - Warp the error with fmt.Errorf() - Rename variables from C style to Golang style - Add blank line. Add CI for macOS and Windows. Fix ldflags for windows. Fix compile errors on windows.
dongbeiouba
force-pushed
the
ci/win_mac
branch
from
348c87a to
ffe81af
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
CI adds golangci-lint check.
Delete duplicate cgo ldflags -lcrypto
Fix golangci-lint issues.
Add CI for macOS and Windows.
Fix ldflags for windows.
Fix compile errors on windows.