Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing accuracy while getting ino (获取ino 精度丢失) #16679

Closed
spiritwindy opened this issue Nov 2, 2017 · 19 comments
Closed

Missing accuracy while getting ino (获取ino 精度丢失) #16679

spiritwindy opened this issue Nov 2, 2017 · 19 comments
Labels
confirmed-bug Issues with confirmed bugs. duplicate Issues and PRs that are duplicates of other issues or PRs. fs Issues and PRs related to the fs subsystem / file system.

Comments

@spiritwindy
Copy link

lalpbbcc1rf0dinnarznawe_353_284 png_620x10000q90g

  • Version:1-9.0
  • Platform:win
  • Subsystem:
    node fs.stat 获取文件 ino 精度丢失
@XadillaX
Copy link
Contributor

XadillaX commented Nov 2, 2017

Translation

Title: Missing accuracy while getting ino

Content:

  • Version: 1-9.0
  • Platform: win
  • Subsystem:

Accuracy lost while getting ino via fs.stat.

@XadillaX XadillaX added the fs Issues and PRs related to the fs subsystem / file system. label Nov 2, 2017
@XadillaX
Copy link
Contributor

XadillaX commented Nov 2, 2017

I think it because the ino in libuv is a unsigned int64:

typedef struct {
    uint64_t st_dev;
    uint64_t st_mode;
    uint64_t st_nlink;
    uint64_t st_uid;
    uint64_t st_gid;
    uint64_t st_rdev;
    uint64_t st_ino;
    uint64_t st_size;
    uint64_t st_blksize;
    uint64_t st_blocks;
    uint64_t st_flags;
    uint64_t st_gen;
    uv_timespec_t st_atim;
    uv_timespec_t st_mtim;
    uv_timespec_t st_ctim;
    uv_timespec_t st_birthtim;
} uv_stat_t;

And when filling to Node.js, it will be changed to double:

void FillStatsArray(double* fields, const uv_stat_t* s) {
  fields[0] = s->st_dev;
  ...

  fields[7] = s->st_ino;
  fields[8] = s->st_size;

  ...
}

Refs

@XadillaX
Copy link
Contributor

XadillaX commented Nov 2, 2017

How about add some function like fs.safeStat and make these value be a buffer or a number string?

@mscdex
Copy link
Contributor

mscdex commented Nov 2, 2017

There is no real solution for this until JS gets BigInt support, which is currently in stage 3. We've had this discussion before, and IMHO we should still wait for BigInt.

@XadillaX
Copy link
Contributor

XadillaX commented Nov 2, 2017

@mscdex but how could @spitWind get it correctly now?

@XadillaX
Copy link
Contributor

XadillaX commented Nov 2, 2017

Or maybe we could provide the raw uv_stat_t buffer to fs.Stats to let them parse by themselves?

@XadillaX XadillaX added the confirmed-bug Issues with confirmed bugs. label Nov 2, 2017
@gireeshpunathil
Copy link
Member

Please clarify: is it just accuracy? As inode numbers are unique identifiers into the disk blocks, I believe any change in the number leads to incorrect and unusable value as opposed to less accurate one?

@XadillaX
Copy link
Contributor

XadillaX commented Nov 2, 2017

@gireeshpunathil That's it. That's why I propose a safer one or a raw one.

@gireeshpunathil
Copy link
Member

@XadillaX - thanks! I Agree that it is a bug that needs to be fixed. If there is no data type to support it, we should not record it in the first place, as not having an info is better than recording wrong info.

@mscdex
Copy link
Contributor

mscdex commented Nov 2, 2017

as not having an info is better than recording wrong info.

I don't really agree with that. It's hard to say how often this is actually a problem in production. This is only the second time anyone has ever brought it up that I've ever seen.

@XadillaX
Copy link
Contributor

XadillaX commented Nov 2, 2017

@spitWind Could you please tell us that the situation you have to get this value in production?

@gireeshpunathil
Copy link
Member

@mscdex - the problem is that it may not manifest as a problem. Wrong but valid inodes can corrupt files inadvertently, and may not be noticed easily.

@bnoordhuis
Copy link
Member

Closing, duplicate of #12115.

@bnoordhuis bnoordhuis added the duplicate Issues and PRs that are duplicates of other issues or PRs. label Nov 2, 2017
@spiritwindy
Copy link
Author

你们可以设置成_int64的,然后获取到原生的buffer比较 buffer就可以确认,,或者字符串

@spiritwindy
Copy link
Author

貌似得解决下~~

@spiritwindy
Copy link
Author

怎么超全局(针对机器)hook node里面的module?比如 fs模块

@spiritwindy
Copy link
Author

两个原因,js 精度。。另外一个是double64..

@tniessen tniessen changed the title 获取ino 精度丢失 Missing accuracy while getting ino (获取ino 精度丢失) Nov 5, 2017
@XadillaX
Copy link
Contributor

XadillaX commented Nov 7, 2017

@spitWind: You may set it to __int64, Buffer or String.

@spitWind: It seems that this really is a bug and should be resolved.

XadillaX added a commit to XadillaX/node that referenced this issue Nov 7, 2017
Since `ino` is an unsigned 64-bit integer, it may lost accuracy in
JavaScript. This situation may cause some strange bugs in filesystem. So
a string type `inoString` is added to `fs.Stats`.

Fixes: nodejs#16679
Fixes: nodejs#12115
Refs: http://docs.libuv.org/en/v1.x/fs.html#c.uv_stat_t
@spiritwindy
Copy link
Author

thank you @XadillaX

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. duplicate Issues and PRs that are duplicates of other issues or PRs. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants