Date: Fri, 03 May 2019 14:36:32 +0000 From: bugzilla-noreply@freebsd.org To: xfce@FreeBSD.org Subject: [Bug 237714] [PATCH] sysutils/xfce4-power-manager: fix craches, improve freebsd support, add DEBUG option Message-ID: <bug-237714-28711-VMHEnikZTM@https.bugs.freebsd.org/bugzilla/> In-Reply-To: <bug-237714-28711@https.bugs.freebsd.org/bugzilla/> References: <bug-237714-28711@https.bugs.freebsd.org/bugzilla/>
next in thread | previous in thread | raw e-mail | index | archive | help
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D237714 Guido Falsi <madpilot@FreeBSD.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|maintainer-feedback?(xfce@F |maintainer-feedback+ |reeBSD.org) | CC| |madpilot@FreeBSD.org Status|New |In Progress --- Comment #1 from Guido Falsi <madpilot@FreeBSD.org> --- Hi, Thanks for the patch. I have some questions: - Why add DEBUG as an option? WITH_DEBUG is not enough to activate it, just adding --enable-debug as needed when WITH_DEBUG is on. While there is no strict rule about this and some ports are adding a DEBUG option, it is not a good practice, and using WITH_DEBUG should be the only = way to enable it. it is not a user flag, it's a developer flag anyway. - Regarding the source changes, I need some clearifications. the g_object_ref change should be upstreamed? Have you already sent a bug report? It looks like a bug to me. Would you propose the patch upstream? I don't get why you need the "return g_strdup (_("Unknown"));" patch. Looki= ng at the code I'd say the intention is for that to never be reached, but maybe there is a bug if 59.0 < value < 60.0. The upstream repository has fixed th= is in a different way. I'd be more comfortable importing the newer upstream co= de [1], to avoid diverging. Could you modify your patch to match upstream code? Fir the xfpm-backlight-helper.c patch, before committing it I'd like to know where the "8" number comes from, and exactly what the patch fixes. This last hunk also contains some gratuitous changes which would make upstreaming it difficult. You should not make whitespace changes (they can = be there for upstream code style rules) and I'd also avoid changing the return logic, and leave it matching what the upstream used. This is because this k= ind of change could be refused by upstream if proposed. Also, while upstream allocating only one gchar looks definitely too little,= 64 is definitely too much, since your code never uses more than 5 (including terminating \0 ). The second argument to g_snprintf could also be calculated correctly. As before, should this be upstreamed? Are you wiling to propose it in an upstream bug report? [1] https://github.com/xfce-mirror/xfce4-power-manager/blob/xfce-4.12/settings/= xfpm-settings.c#L1567 --=20 You are receiving this mail because: You are the assignee for the bug.=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-237714-28711-VMHEnikZTM>