python · vstinner · Jun 1, 2020 · Feb 9, 2020 · Feb 10, 2020 · May 27, 2020
diff --git a/Lib/ctypes/test/test_struct_fields.py b/Lib/ctypes/test/test_struct_fields.py
@@ -46,6 +46,14 @@ class Y(X):
        Y._fields_ = []
        self.assertRaises(AttributeError, setattr, X, "_fields_", [])

+    def test_5(self):
+        class X(Structure):
+            _fields_ = (("char", c_char * 5),)
+
+        x = X(b'#' * 5)
+        x.char = b'a\0b\0'
+        self.assertEqual(bytes(x), b'a\x00###')
+
    # __set__ and __get__ should raise a TypeError in case their self
    # argument is not a ctype instance.
    def test___set__(self):

diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c
@@ -1289,7 +1289,9 @@ s_set(void *ptr, PyObject *value, Py_ssize_t length)
    }

    data = PyBytes_AS_STRING(value);
-    size = strlen(data); /* XXX Why not Py_SIZE(value)? */
+    /* bpo-39593: XXX Why not Py_SIZE(value)? */
+    size = strlen(data);
+
    if (size < length) {
        /* This will copy the terminating NUL character
         * if there is space for it.
-Original file line number
+Diff line change
@@ -1289,7 +1289,9 @@ s_set(void *ptr, PyObject *value, Py_ssize_t length)
         }
         data = PyBytes_AS_STRING(value);
-        size = strlen(data); /* XXX Why not Py_SIZE(value)? */
+        /* bpo-39593: XXX Why not Py_SIZE(value)? */
             Copy link

  
      
    
  

  
      

  
  Member


      

  

  
    
      

      
            vstinner
  

      

      

      


        May 28, 2020


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

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

    
      
      


  


  
    
      I don't think that it's correct to add a test which indirectly specify the behavior and keep this comment which suggests to change the behavior.
I suggest to rewrite the comment to explain that we do use strlen() on purpose, rather than PyBytes_GET_SIZE() since we truncate characters after a null character on purpose. Keep the bpo number in the comment.
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
  


          
          
        
      
    

    



  
        
  
    
        
    
  


      
          
  
      
            Copy link

  
      
    
  

  
      

  
  Member


      

  Author


  

  
    
      

      
            shihai1991
  

      

      

      


        May 28, 2020


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

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

    
      
      


  


  
    
      Thanks, got it. I updated the description info, do you think it's clear enough?
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
+        size = strlen(data);
                   shihai1991 marked this conversation as resolved.
              

          
            Show resolved
            Hide resolved
        

                
  
            
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.
         if (size < length) {
             /* This will copy the terminating NUL character
              * if there is space for it.
-          Expand Down