I am closing the mmap object, still the test is failing

Let's wait CI/CD results. Also, can you add a few lines about this inNEWS entry?

Great work@Agent-Hellboy !
Let's see what core devs will say.

still some cases not covered properly. Now that you moved theCHECK_VALID(NULL); into the if-branch, you don't check whether the mmap is closed any more if the index is a slice. try adding a test like:

m=mmap(...)m.close()m[0:5]# should complain about a closed file

also you should add a test that uses the weirdX class as part of a slice

m[X():5]

and you need to do the same thing again for assignment, both withX as an index and a slice:

m[X()]=1...m[X():5]=b'abcde'

or something like that.

Also there needs to be a test for the special case I mentioned at the end of the issue:

m=mmap(...)m.close()withself.assertRaises(ValueError):m["abc"]# not TypeError!

hi@cfbolz, Now I am checking at the start as well which denies throwing TypeError for closed mmap

now you need to do the same things for item assignments still.

We shouldCHECK_VALID(NULL); before eachself->data access, unless we have strong proof that our handle is not closed.

@Agent-Hellboy

I am saying you should add these

elseif (PySlice_Check(item)) {Py_ssize_tstart,stop,step,slicelen;if (PySlice_Unpack(item,&start,&stop,&step)<0) {returnNULL;        }CHECK_VALID(NULL);/////////////////////////////////////////////////////////////////slicelen=PySlice_AdjustIndices(self->size,&start,&stop,step);if (slicelen <=0)returnPyBytes_FromStringAndSize("",0);elseif (step==1)returnPyBytes_FromStringAndSize(self->data+start,slicelen);else {char*result_buf= (char*)PyMem_Malloc(slicelen);size_tcur;Py_ssize_ti;PyObject*result;if (result_buf==NULL)returnPyErr_NoMemory();CHECK_VALID(NULL);/////////////////////////////////////////////////////////////////for (cur=start,i=0;i<slicelen;cur+=step,i++) {result_buf[i]=self->data[cur];            }result=PyBytes_FromStringAndSize(result_buf,slicelen);PyMem_Free(result_buf);returnresult;        }    }else {PyErr_SetString(PyExc_TypeError,"mmap indices must be integers");returnNULL;    }}

Hi@sunmy2019, please share with me a scenario where it's bypassing the firstCHECK_VALID(NULL); at the start for a slice

Hi@sunmy2019, please share with me a scenario where it's bypassing the firstCHECK_VALID(NULL); at the start for a slice

m[X():5]

Currently, it's throwing expected Value error, i have added a test for the same

Currently, it's throwing expected Value error

This is wrong. And you are not creating a newm, which covered your mistake.

then we access self->fd

I think checkingself->fd != -1 is enough here.

I think it's now ready for review.@JelleZijlstra@cfbolz

Sorry for the delay!

Thanks@Agent-Hellboy for the PR, and@JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

GH-104677 is a backport of this pull request to the3.11 branch.