<AWT Dev> [9] Review request: 8040007 GtkFileDialog strips user inputted filepath

[Anthony Petrov]

Hi Alexander,

src/solaris/native/sun/awt/sun_awt_X11_GtkFileDialogPeer.c
>  176         free(prevDir);
>  177         prevDir = strdup(dir);

It's unnecessary to re-duplicate the prevDir on each loop iteration 
here. I suggest to initialize it once instead. The less pointer 
operations, the less room for bugs.


>  229         (*env)->ExceptionClear(env);
>  230         JNU_ThrowInternalError(env, "Could not instantiate current folder");

This error message sounds misleading because the code above doesn't 
instantiate a "folder", it tries to instantiate a string. Also, if 
exceptions did occur, I believe it's fine to report them as they are. 
Otherwise, if the pointer is NULL, then this looks more like an OOM than 
an internal error to me.


>  278         //This is a hack for use with "Recent Folders" in gtk where each
>  279         //file could have its own directory.

I assume you've tested this case, and it still works fine, right?

--
best regards,
Anthony

On 6/6/2014 8:00 PM, Alexander Zvegintsev wrote:
> Hello,
>
> please review the fix
> http://cr.openjdk.java.net/~azvegint/jdk/9/8040007/00/
> for the bug
> https://bugs.openjdk.java.net/browse/JDK-8040007
>
> With this fix we don't use current folder from native GtkFileChooser
> anymore, now we build it by ourselves.
>
> [1]
> https://developer.gnome.org/gtk2/stable/GtkFileChooser.html#gtk-file-chooser-get-filenames
>
> [2]
> https://developer.gnome.org/glib/stable/glib-Miscellaneous-Utility-Functions.html#g-path-get-dirname
>
>