search

kata-containers / kata-containers

Kata Containers is an open source project and community working to build a standard implementation of lightweight Virtual Machines (VMs) that feel and perform like containers, but provide the workload isolation and security advantages of VMs. https://katacontainers.io/
Apache License 2.0
5.08k stars 1k forks source link

runtime-rs: fix the bug of func count_files #9830

Closed gaohuatao-1 closed 1 day ago

gaohuatao-1 commented 2 weeks ago

When the total number of files observed is greater than limit, return -1 directly. runtime has fixed this bug, it should b ported to runtime-rs.

Fixes:#9829

justxuewei commented 2 weeks ago

/test

justxuewei commented 2 weeks ago

@gaohuatao-1 Thanks for bringing this to runtime-rs. Could you mind adding some unit tests to the method?

justxuewei commented 1 week ago

/test

gaohuatao-1 commented 1 week ago

@justxuewei @Apokleos I have added a ut to the method. PTAL again, thanks.

justxuewei commented 1 week ago

/test

gaohuatao-1 commented 1 week ago

@Apokleos Hi, PTAL, Thanks.

gaohuatao-1 commented 1 week ago

Thx @gaohuatao-1 Compared with CountFiles in kata runtime with golang, this commit looks good to me. But I try to set the limit < 0 (limit declared as i32), the UT fails. I think the argument limit: i32 in count_files() shoud be change as limit: u32, maybe it's another bug, what do you think ? @Apokleos
Thanks for your reply. I did the test: the input param limit of func count_files is smaller than zero and test_count_files is still passed. such as: image

Could you please take a look again? Thanks a lot.

Apokleos commented 1 day ago

Thx @gaohuatao-1 Compared with CountFiles in kata runtime with golang, this commit looks good to me. But I try to set the limit < 0 (limit declared as i32), the UT fails. I think the argument limit: i32 in count_files() shoud be change as limit: u32, maybe it's another bug, what do you think ? @Apokleos Thanks for your reply. I did the test: the input param limit of func count_files is smaller than zero and test_count_files is still passed. such as: image

Could you please take a look again? Thanks a lot.

what's about -2 ? IMO, if you set -1 the return value is -1 which looks same with the Ok(-1), but they are in fact not same, I think.

gaohuatao-1 commented 1 day ago

Thx @gaohuatao-1 Compared with CountFiles in kata runtime with golang, this commit looks good to me. But I try to set the limit < 0 (limit declared as i32), the UT fails. I think the argument limit: i32 in count_files() shoud be change as limit: u32, maybe it's another bug, what do you think ? @Apokleos Thanks for your reply. I did the test: the input param limit of func count_files is smaller than zero and test_count_files is still passed. such as: image

Could you please take a look again? Thanks a lot.

what's about -2 ? IMO, if you set -1 the return value is -1 which looks same with the Ok(-1), but they are in fact not same, I think. -2 is ok, too. such as: image @Apokleos