Deprecate BSTR String constructor and setValue (#1300)

BSTR allocation should be managed by COM, Automation, and Interop functions.  See https://docs.microsoft.com/en-us/previous-versions/windows/desktop/automat/bstr

CHANGES.md

@@ -31,6 +31,8 @@ Bug Fixes
 * [#1279](https://github.com/java-native-access/jna/issues/1279): Remove `DLLCallback` import from `CallbackReference` - [@dyorgio](https://github.com/dyorgio).
 * [#1278](https://github.com/java-native-access/jna/pull/1278): Improve compatibility of `c.s.j.p.WindowUtils#getProcessFilePath` and fix unittests for windows 32bit intel  - [@matthiasblaesing](https://github.com/matthiasblaesing).
 * [#1284](https://github.com/java-native-access/jna/pull/1284): Fix illegal access exceptions, when retrieving options for private library interfaces with an instance field - [@fkistner](https://github.com/fkistner).
+* [#1300](https://github.com/java-native-access/jna/pull/1300): Deprecate `c.s.j.p.win32.WTypes.BSTR` String constructor and `setValue` method, as `BSTR` allocation should be managed by COM, Automation, and Interop functions - [@dbwiddis](https://github.com/dbwiddis).
+
 
 Breaking Changes
 ----------------

contrib/platform/src/com/sun/jna/platform/win32/WTypes.java

@@ -24,14 +24,15 @@
  */
 package com.sun.jna.platform.win32;
 
+import java.io.UnsupportedEncodingException;
+
 import com.sun.jna.Memory;
 import com.sun.jna.Native;
 import com.sun.jna.Pointer;
 import com.sun.jna.PointerType;
 import com.sun.jna.Structure;
 import com.sun.jna.platform.win32.WinDef.USHORT;
 import com.sun.jna.ptr.ByReference;
-import java.io.UnsupportedEncodingException;
 
 /**
  * Constant defined in WTypes.h
@@ -103,15 +104,33 @@ public BSTR() {
             super(Pointer.NULL);
         }
 
+        /**
+         * Instantiate a BSTR from a pointer. The user is responsible for allocating and
+         * releasing memory for the {@link BSTR}, most commonly using
+         * {@link OleAuto#SysAllocString(String)} and
+         * {@link OleAuto#SysFreeString(BSTR)}
+         *
+         * @param pointer
+         *            A pointer to the string
+         */
         public BSTR(Pointer pointer) {
             super(pointer);
         }
 
+        /**
+         * @deprecated Use {@link OleAuto#SysAllocString(String)} and
+         *             {@link OleAuto#SysFreeString(BSTR)}
+         */
+        @Deprecated
         public BSTR(String value) {
             super();
             this.setValue(value);
         }
 
+        /**
+         * @deprecated Users should not change the value of an allocated {@link BSTR}.
+         */
+        @Deprecated
         public void setValue(String value) {
             if(value == null) {
                 value = "";
@@ -154,21 +173,56 @@ public BSTRByReference() {
             super(Native.POINTER_SIZE);
         }
 
+        /**
+         * Store a reference to the specified {@link BSTR}. This method does not
+         * maintain a reference to the object passed as an argument. The user is
+         * responsible for allocating and freeing the memory associated with this
+         * {@link BSTR}.
+         *
+         * @param value
+         *            The BSTR to be referenced. Only the pointer is stored as a
+         *            reference.
+         */
         public BSTRByReference(BSTR value) {
             this();
             setValue(value);
         }
 
+        /**
+         * Store a reference to the specified {@link BSTR}. This method does not
+         * maintain a reference to the object passed as an argument. The user is
+         * responsible for allocating and freeing the memory associated with this
+         * {@link BSTR}.
+         *
+         * @param value
+         *            The BSTR to be referenced. Only the pointer is stored as a
+         *            reference.
+         */
         public void setValue(BSTR value) {
             this.getPointer().setPointer(0, value.getPointer());
         }
 
+        /**
+         * Returns a copy of the {@link BSTR} referenced by this object. The memory
+         * associated with the {@link BSTR} may be referenced by other objects/threads
+         * which may also free the underlying native memory.
+         *
+         * @return A new {@link BSTR} object corresponding to the memory referenced by
+         *         this object.
+         */
         public BSTR getValue() {
             return new BSTR(getPointer().getPointer(0));
         }
 
+        /**
+         * Returns the String represented by the referenced {@link BSTR}.
+         *
+         * @return the referenced String, if the reference is not {@code null},
+         *         {@code null} otherwise.
+         */
         public String getString() {
-            return this.getValue().getValue();
+            BSTR b = this.getValue();
+            return b == null ? null : b.getValue();
         }
     }
 

contrib/platform/test/com/sun/jna/platform/ByReferencePlatformToStringTest.java

@@ -28,6 +28,7 @@
 
 import org.junit.Test;
 
+import com.sun.jna.Platform;
 import com.sun.jna.Pointer;
 import com.sun.jna.platform.unix.X11.AtomByReference;
 import com.sun.jna.platform.unix.X11.WindowByReference;
@@ -43,6 +44,7 @@
 import com.sun.jna.platform.win32.OaIdl.VARIANT_BOOL;
 import com.sun.jna.platform.win32.OaIdl.VARIANT_BOOLByReference;
 import com.sun.jna.platform.win32.OaIdl._VARIANT_BOOLByReference;
+import com.sun.jna.platform.win32.OleAuto;
 import com.sun.jna.platform.win32.WTypes.BSTR;
 import com.sun.jna.platform.win32.WTypes.BSTRByReference;
 import com.sun.jna.platform.win32.WTypes.VARTYPE;
@@ -84,8 +86,12 @@ public void testPlatformToStrings() {
         BOOLByReference boolbr = new BOOLByReference(new BOOL(true));
         parseAndTest(boolbr.toString(), "BOOL", "true");
 
-        BSTRByReference bstrbr = new BSTRByReference(new BSTR("bstr"));
-        parseAndTest(bstrbr.toString(), "BSTR", "bstr");
+        if (Platform.isWindows()) {
+            BSTR b = OleAuto.INSTANCE.SysAllocString("bstr");
+            BSTRByReference bstrbr = new BSTRByReference(b);
+            parseAndTest(bstrbr.toString(), "BSTR", "bstr");
+            OleAuto.INSTANCE.SysFreeString(b);
+        }
 
         CHARByReference cbr = new CHARByReference(new CHAR(42));
         parseAndTest(cbr.toString(), "CHAR", "42");