[PUSHED] Re: [REVIEW] Null pointer passed as an argument to a 'nonnull' parameter in vcl/unx/generic/app/i18n_wrp.cxx
Hi, The John Smith's clang report reminded me this thread. I pushed this easy fix on master (see http://cgit.freedesktop.org/libreoffice/core/commit/?id=4de603d28ce884fb2a3720b6d132465b6dcc98a9). I don't know if this part of code is or will be used but it can't do any harm. Julien -- View this message in context: http://nabble.documentfoundation.org/REVIEW-Null-pointer-passed-as-an-argument-to-a-nonnull-parameter-in-vcl-unx-generic-app-i18n-wrp-cxx-tp3752158p3999352.html Sent from the Dev mailing list archive at Nabble.com. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [PUSHED] Re: [REVIEW] Null pointer passed as an argument to a 'nonnull' parameter in vcl/unx/generic/app/i18n_wrp.cxx
On Sun, Aug 5, 2012 at 1:55 PM, julien2412 serval2...@yahoo.fr wrote: Hi, The John Smith's clang report reminded me this thread. I pushed this easy fix on master (see http://cgit.freedesktop.org/libreoffice/core/commit/?id=4de603d28ce884fb2a3720b6d132465b6dcc98a9). I don't know if this part of code is or will be used but it can't do any harm. Julien That's great! It's nice to see people really taking the time and effort to look into that clang report. - John ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [REVIEW] Null pointer passed as an argument to a 'nonnull' parameter in vcl/unx/generic/app/i18n_wrp.cxx
On 02/16/2012 11:39 PM, julien2412 wrote: /home/julien/compile-libreoffice/libo/vcl/unx/generic/app/i18n_wrp.cxx:250:9: warning: Null pointer passed as an argument to a 'nonnull' parameter dlclose(g_dlmodule); ^ ~~ 1 warning generated. Here are the lines : 243 Status XvaCloseIM(XIM) [...] Remark : when I opengroked this function , I found nothing. Is this function used (or should be used) in a way ? Looks like all of vcl/unx/generic/app/i18n_wrp.cxx became completely unused with http://cgit.freedesktop.org/libreoffice/core/commit/?id=8b0287543d87659fd4bfde5edb6725ee5da5f80e These multi-lingual IMs have been always disabled for Linux. So I would suggest to remove that code wholesale. Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [REVIEW] Null pointer passed as an argument to a 'nonnull' parameter in vcl/unx/generic/app/i18n_wrp.cxx
So I would suggest to remove that code wholesale. Hmm, but was this code used on Solaris? Have we already removed some functionality needed (or at least useful) on Solaris then? Too bad as there recently has been people here talking about supporting Solaris? (When I say Solaris, I mean whichever of them somebody wants to support...) --tml ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [REVIEW] Null pointer passed as an argument to a 'nonnull' parameter in vcl/unx/generic/app/i18n_wrp.cxx
On 02/17/2012 09:53 AM, Tor Lillqvist wrote: So I would suggest to remove that code wholesale. Hmm, but was this code used on Solaris? Have we already removed some functionality needed (or at least useful) on Solaris then? Too bad as there recently has been people here talking about supporting Solaris? (When I say Solaris, I mean whichever of them somebody wants to support...) But even then, leaving in just half of it while the rest has already been removed would not make much sense IMO. Stephan ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [REVIEW] Null pointer passed as an argument to a 'nonnull' parameter in vcl/unx/generic/app/i18n_wrp.cxx
But even then, leaving in just half of it while the rest has already been removed would not make much sense IMO. Indeed, so should the earlier removal be reverted? --tml ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [REVIEW] Null pointer passed as an argument to a 'nonnull' parameter in vcl/unx/generic/app/i18n_wrp.cxx
On Fri, 2012-02-17 at 12:16 +0200, Tor Lillqvist wrote: But even then, leaving in just half of it while the rest has already been removed would not make much sense IMO. Indeed, so should the earlier removal be reverted? If people want to make Solaris (or it's more viable free software brethren) work they're more than welcome. In the meantime, having un-tested, un-executed, legacy code around for a platform we don't support seems pretty pointless to me :-) All the best, Michael. -- michael.me...@suse.com , Pseudo Engineer, itinerant idiot ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [REVIEW] Null pointer passed as an argument to a 'nonnull' parameter in vcl/unx/generic/app/i18n_wrp.cxx
In the meantime, having un-tested, un-executed, legacy code around for a platform we don't support seems pretty pointless to me :-) What message does that send to the Solaris porters? --tml ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [REVIEW] Null pointer passed as an argument to a 'nonnull' parameter in vcl/unx/generic/app/i18n_wrp.cxx
On Fri, 2012-02-17 at 13:21 +0200, Tor Lillqvist wrote: In the meantime, having un-tested, un-executed, legacy code around for a platform we don't support seems pretty pointless to me :-) What message does that send to the Solaris porters? To pull their finger out and get it building on OpenIndiana / Illumos / whatever quickish ? :-) I'm very happy to help do the (fairly trivial) grep for 'SOLARIS' in the git log -u output - to find the bits that have been removed that they might want to help restore it later if that's what they want. All the best, Michael. -- michael.me...@suse.com , Pseudo Engineer, itinerant idiot ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [REVIEW] Null pointer passed as an argument to a 'nonnull' parameter in vcl/unx/generic/app/i18n_wrp.cxx
On Fri, 2012-02-17 at 10:53 +0200, Tor Lillqvist wrote: So I would suggest to remove that code wholesale. Hmm, but was this code used on Solaris? Have we already removed some functionality needed (or at least useful) on Solaris then? I doubt it, solaris had moved to a GNOME desktop by default, more or less, when I left Sun and last used solaris and all of this is irrelevant under the gtk vclplug. It doesn't really make a lot of sense to retain direct support for one archaic Input Mechanism which has been superseded, a few time over by now, by a series of new IMs. IIIMP support probably shouldn't have been made solaris-specific in the first place as e.g. fedora had it in up until Fedora 5 in 2006 or so. In fact we should probably go so far as to drop the generic vcl plug altogether and just have the gtk and kde ones. C. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [REVIEW] Null pointer passed as an argument to a 'nonnull' parameter in vcl/unx/generic/app/i18n_wrp.cxx
On Fri, 2012-02-17 at 14:05 +, Caolán McNamara wrote: In fact we should probably go so far as to drop the generic vcl plug altogether and just have the gtk and kde ones. An interesting idea; might be nice to do, though of course, since so much code is shared anyway between gtk2, kde and gen - I don't know how much we'd actually save (?). ATB, Michael. -- michael.me...@suse.com , Pseudo Engineer, itinerant idiot ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [REVIEW] Null pointer passed as an argument to a 'nonnull' parameter in vcl/unx/generic/app/i18n_wrp.cxx
On Fri, 2012-02-17 at 14:34 +, Michael Meeks wrote: On Fri, 2012-02-17 at 14:05 +, Caolán McNamara wrote: In fact we should probably go so far as to drop the generic vcl plug altogether and just have the gtk and kde ones. An interesting idea; might be nice to do, though of course, since so much code is shared anyway between gtk2, kde and gen - I don't know how much we'd actually save (?). We'd probably save very little from a size perspective, though perhaps gain a bit from a simplicity perspective. Though on the other hand, maybe its still convenient to have a more basic plugin to refer to sometimes. Dunno, I'm easy either way. C. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [REVIEW] Null pointer passed as an argument to a 'nonnull' parameter in vcl/unx/generic/app/i18n_wrp.cxx
Hello, Il 17/02/2012 15:39, Caolán McNamara ha scritto: On Fri, 2012-02-17 at 14:34 +, Michael Meeks wrote: On Fri, 2012-02-17 at 14:05 +, Caolán McNamara wrote: In fact we should probably go so far as to drop the generic vcl plug altogether and just have the gtk and kde ones. An interesting idea; might be nice to do, though of course, since so much code is shared anyway between gtk2, kde and gen - I don't know how much we'd actually save (?). We'd probably save very little from a size perspective, though perhaps gain a bit from a simplicity perspective. Though on the other hand, maybe its still convenient to have a more basic plugin to refer to sometimes. Dunno, I'm easy either way. FYI, I'm actually using these two files to build headless. vcl/unx/generic/printer/jobdata \ vcl/unx/generic/printer/ppdparser \ I don't actually need printing but maybe other interested people do? thanks -- Riccardo Magliocchetti ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [REVIEW] Null pointer passed as an argument to a 'nonnull' parameter in vcl/unx/generic/app/i18n_wrp.cxx
I doubt it, solaris had moved to a GNOME desktop by default, more or less, when I left Sun and last used solaris and all of this is irrelevant under the gtk vclplug. OK, thanks for the technical explanation, if the code in question is obsolete on any reasonable Solaris platform (whose users might be interested in LibreOffice in the first place) then I of course have no objection. --tml ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [REVIEW] Null pointer passed as an argument to a 'nonnull' parameter in vcl/unx/generic/app/i18n_wrp.cxx
On Fri, 2012-02-17 at 16:06 +0100, Riccardo Magliocchetti wrote: FYI, I'm actually using these two files to build headless. Which reminds me - I should review your patch ;-) sorry ... vcl/unx/generic/printer/jobdata \ vcl/unx/generic/printer/ppdparser \ I don't actually need printing but maybe other interested people do? Ah - so, if these are suitably generic, or can be made so, perhaps we should whack them into vcl/generic/ instead. The vcl/unx/generic/ is rather unix specific, but then some of the generic/ stuff is overly unix specific too still, but ... :-) There's plenty of cleanup / re-factoring dependency reduction required in the vcl/ code yet. ATB, Michael. -- michael.me...@suse.com , Pseudo Engineer, itinerant idiot ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[REVIEW] Null pointer passed as an argument to a 'nonnull' parameter in vcl/unx/generic/app/i18n_wrp.cxx
Hello,
By keeping on reading the logs cclang compilation, I noticed this :
/home/julien/compile-libreoffice/libo/vcl/unx/generic/app/i18n_wrp.cxx:250:9:
warning: Null pointer passed as an argument to a 'nonnull' parameter
dlclose(g_dlmodule);
^ ~~
1 warning generated.
Here are the lines :
243 Status XvaCloseIM(XIM)
244 {
245 Status s = False;
246
247 if (!g_dlmodule) CAUSE OF THE PB
248 {
249 /* assuming one XvaOpenIM call */
250 dlclose(g_dlmodule); HERE
251 g_dlmodule = (void*)0;
252 g_open_im = (OpenFunction)NULL;
253 s = True;
254 }
255 return (s);
256 }
So here's an obvious patch :
diff --git a/vcl/unx/generic/app/i18n_wrp.cxx
b/vcl/unx/generic/app/i18n_wrp.cxx
index 3aff25c..b95ad21 100644
--- a/vcl/unx/generic/app/i18n_wrp.cxx
+++ b/vcl/unx/generic/app/i18n_wrp.cxx
@@ -244,7 +244,7 @@ Status XvaCloseIM(XIM)
{
Status s = False;
-if (!g_dlmodule)
+if (g_dlmodule)
{
/* assuming one XvaOpenIM call */
dlclose(g_dlmodule);
I suppose again it's ok to push this fix on master but would it be useful to
push this on 3.5 branch too ?
Remark : when I opengroked this function , I found nothing. Is this function
used (or should be used) in a way ?
Julien.
--
View this message in context:
http://nabble.documentfoundation.org/REVIEW-Null-pointer-passed-as-an-argument-to-a-nonnull-parameter-in-vcl-unx-generic-app-i18n-wrp-cxx-tp3752158p3752158.html
Sent from the Dev mailing list archive at Nabble.com.
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
