After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 734977 - Make rsvg-convert work again on Windows for stdin input
Make rsvg-convert work again on Windows for stdin input
Status: RESOLVED FIXED
Product: librsvg
Classification: Core
Component: general
git master
Other Windows
: Normal normal
: ---
Assigned To: librsvg maintainers
librsvg maintainers
Depends on:
Blocks:
 
 
Reported: 2014-08-18 04:42 UTC by Fan, Chun-wei
Modified: 2015-02-14 04:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Use GIO-Win32 API to regain support for Win32 (2.08 KB, patch)
2014-08-18 04:43 UTC, Fan, Chun-wei
none Details | Review
Patch to fix build and use of rsvg-convert on Windows (2.38 KB, patch)
2014-12-19 10:27 UTC, Fan, Chun-wei
needs-work Details | Review
Patch to fix build and use of rsvg-convert on Windows (take ii) (2.32 KB, patch)
2014-12-19 10:46 UTC, Fan, Chun-wei
committed Details | Review

Description Fan, Chun-wei 2014-08-18 04:42:07 UTC
Hi,

In the works to support huge SVG XML parsing, g_unix_input_stream_new() was used, which made the program not buildable on Windows.  This bug is to have the tool re-support Windows.

With blessings, thank you!
Comment 1 Fan, Chun-wei 2014-08-18 04:43:55 UTC
Created attachment 283703 [details] [review]
Use GIO-Win32 API to regain support for Win32

Hi,

This is my patch to use GIO-Win32 APIs to regain support for this tool on Windows, to be able to read input from STDIN on Windows.

With blessings, thank you!
Comment 2 Fan, Chun-wei 2014-12-19 09:32:34 UTC
Hi,

This bug has been fixed upstream by commit 1811f20, so I will close this now.

With blessings, thank you!
Comment 3 Fan, Chun-wei 2014-12-19 10:14:01 UTC
Hi,

Sorry, unfortunately I have to re-open this bug so soon, as commmit 1811f20 didn't fix the use of rsvg-convert on Windows correctly (I blindly closed this bug as I thought that commit fixed this correctly), as g_win32_input_stream_new() expects a HANDLE as the first parameter, not a special constant like STDIN_FILENO.

I will attach a new version of the patch for it, based on the latest git master checkout.

With blessings, thank you!
Comment 4 Fan, Chun-wei 2014-12-19 10:27:39 UTC
Created attachment 293034 [details] [review]
Patch to fix build and use of rsvg-convert on Windows

Hi,

This is my fix for this.

With blessings, thank you!
Comment 5 Fan, Chun-wei 2014-12-19 10:33:17 UTC
Hi,

Adding Nacho to the CC list as well, as he might want a look at this.

With blessings.
Comment 6 Ignacio Casal Quinteiro (nacho) 2014-12-19 10:38:10 UTC
Review of attachment 293034 [details] [review]:

See the comments.

::: rsvg-convert.c
@@ +128,3 @@
 
+#ifdef G_OS_WIN32
+    gpointer handle;

should this be just HANDLE?

@@ +232,3 @@
+
+            if (handle == INVALID_HANDLE_VALUE) {
+              gchar* emsg = g_win32_error_message (GetLastError());

it should be gchar *emsg

@@ +234,3 @@
+              gchar* emsg = g_win32_error_message (GetLastError());
+              fprintf (stderr, _("Unable to acquire HANDLE for STDIN. Error was:\n"));
+              fprintf (stderr, "%s\n", emsg);

use g_printerr here and maybe change it to:
g_printerr ( _("Unable to acquire HANDLE for STDIN: %s\n"), emsg);
Comment 7 Fan, Chun-wei 2014-12-19 10:46:17 UTC
Created attachment 293038 [details] [review]
Patch to fix build and use of rsvg-convert on Windows (take ii)

Hello Nacho,

Thanks for the reviews...

(In reply to comment #6)

> +#ifdef G_OS_WIN32
> +    gpointer handle;
> 
> should this be just HANDLE?

Ideally yes, so I updated it here.  It's interesting to see that the prototype of g_win32_input_stream_new() wanted a void*, so I followed it initially, oh well (HANDLE is typedef'ed as

> it should be gchar *emsg

Got it.

> use g_printerr here and maybe change it to:
> g_printerr ( _("Unable to acquire HANDLE for STDIN: %s\n"), emsg);

Got it.

With blessings, thank you!
Comment 8 Fan, Chun-wei 2014-12-19 10:47:19 UTC
("HANDLE is typedef'ed as" should read "HANDLE is typedef'ed as void*"), typed ahead of myself...
Comment 9 Ignacio Casal Quinteiro (nacho) 2014-12-19 10:56:37 UTC
Review of attachment 293038 [details] [review]:

See the comment.

::: rsvg-convert.c
@@ +237,3 @@
+              exit (1);
+            }
+            stream = g_win32_input_stream_new (handle, FALSE);

does the compiler complains here about the missing cast? if yes add (void *) here if not feel free to push it
Comment 10 Fan, Chun-wei 2014-12-19 11:08:34 UTC
Review of attachment 293038 [details] [review]:

Hello Nacho,

The compiler only had a few C4996 (use of deprecated APIs), which means this patch by itself had no warning, so I went ahead to push the patch (as 452ef81).

Thanks for the reviews, with blessings.
Comment 11 Bakhtiar Hasmanan 2015-02-14 04:33:19 UTC
Might better to see gio\tests\win32-streams.c how to implement this.