Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

filesystem: bug fix & code refactor #148

Merged
merged 3 commits into from
Aug 8, 2018
Merged

Conversation

shengofsun
Copy link
Contributor

  1. fix thread safety problem of strerror
  2. remove source code for windows

@shengofsun shengofsun requested review from qinzuoyan, 54chen, neverchanje and acelyc111 and removed request for 54chen August 6, 2018 12:11
1. fix thread safety problem of strerror
2. remove code for windows
!defined _GNU_SOURCE)
int result = strerror_r(err, errno_buffer, 256);
if (result != 0) {
printf("call strerror_r faild, old_err(%d), new_err(%d), ret(%d)\n", err, errno, result);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不应该用 printf

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

目前是在避免采用dlog系列。dlog系列实在是太毒瘤了,引用了就要依赖整个runtime

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

那也应该用 fprintf(stderr, ) 啊,而且正常的基础库都会依赖一个日志库的,你也不能因为避免打日志就用 printf

@neverchanje
Copy link
Contributor

neverchanje commented Aug 7, 2018

safe_strerror 这个函数是自己写的还是从别的地方搞过来的?如果是复制过来的建议能注释一下出处,以后方便跟踪。如果是自己写的麻烦加个注释。

另外 butil 了解一下:
https://github.com/brpc/brpc/blob/master/src/butil/safe_strerror_posix.h
https://github.com/brpc/brpc/blob/master/src/butil/safe_strerror_posix.cc

@shengofsun
Copy link
Contributor Author

safe_strerror是我自己写的……

@shengofsun
Copy link
Contributor Author

butin的safe_strerror内存拷贝次数有点多啊。不过虽然也不要紧,因为打印这个的时候都是出问题了的时候。

@neverchanje
Copy link
Contributor

butil 拷贝再多也只是 256 bytes 的拷贝,我推荐这个主要还是因为它们从 chromium 拷过来的,注释写的比较充分,理论上也比较 stable。

@XiaoMi XiaoMi deleted a comment from neverchanje Aug 8, 2018
@XiaoMi XiaoMi deleted a comment from shengofsun Aug 8, 2018
@qinzuoyan qinzuoyan merged commit f53c6cf into XiaoMi:master Aug 8, 2018
@qinzuoyan qinzuoyan mentioned this pull request Aug 20, 2018
28 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type/bug-fix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants