Re: fix gkrellmreminder (was Re: Port: gkrellmreminder-2.0.0p10)

2022-01-21 Thread Omar Polo
Theo Buehler  writes:

> On Fri, Jan 21, 2022 at 07:00:52PM +0100, Omar Polo wrote:
>> Hello,
>> 
>> The reminder plugin for gkrellm is broken, and has been for a long time
>> apparently.  It stores the reminders in a file and uses fscanf with %d
>> into a time_t to read the date, which corrupts it.  The invalid time is
>> later passed to localtime(3) which returns NULL and the plugin crashes a
>> bit later into strftime.
>> 
>> The diff below is an attempt to fix the port.  Is it fine to read
>> directly a time_t with %lld or should I introduce a temp variable for
>> that?
>
> I would say it's better to use a temporary long long, then assign the
> result to time_t with inevitable truncation on platforms without 64-bit
> time_t. On such platforms, scanf with %lld to a time_t would be
> undefined behavior.
>
> From the high revision number and the dead HOMEPAGE I deduce that we
> won't be able to upstream this, so I suppose your approach is good
> enough.
>
> Either way ok

I opted for using a temporary variable and added a comment before the
patch explaining the issue, thanks!



Re: fix gkrellmreminder (was Re: Port: gkrellmreminder-2.0.0p10)

2022-01-21 Thread Theo Buehler
On Fri, Jan 21, 2022 at 07:00:52PM +0100, Omar Polo wrote:
> Hello,
> 
> The reminder plugin for gkrellm is broken, and has been for a long time
> apparently.  It stores the reminders in a file and uses fscanf with %d
> into a time_t to read the date, which corrupts it.  The invalid time is
> later passed to localtime(3) which returns NULL and the plugin crashes a
> bit later into strftime.
> 
> The diff below is an attempt to fix the port.  Is it fine to read
> directly a time_t with %lld or should I introduce a temp variable for
> that?

I would say it's better to use a temporary long long, then assign the
result to time_t with inevitable truncation on platforms without 64-bit
time_t. On such platforms, scanf with %lld to a time_t would be
undefined behavior.

>From the high revision number and the dead HOMEPAGE I deduce that we
won't be able to upstream this, so I suppose your approach is good
enough.

Either way ok



fix gkrellmreminder (was Re: Port: gkrellmreminder-2.0.0p10)

2022-01-21 Thread Omar Polo
Hello,

The reminder plugin for gkrellm is broken, and has been for a long time
apparently.  It stores the reminders in a file and uses fscanf with %d
into a time_t to read the date, which corrupts it.  The invalid time is
later passed to localtime(3) which returns NULL and the plugin crashes a
bit later into strftime.

The diff below is an attempt to fix the port.  Is it fine to read
directly a time_t with %lld or should I introduce a temp variable for
that?

With this I can successfully add and see reminders without having the
program crashing.  To see the crash, just enable the grkellmreminder
plugin, click in the reminder widget, schedule an event, press "add" and
then "apply."

This was also tested by John McCue (+cc) who reported the problem in the
first place.

OK?  (the alternative would be dropping the port I guess)


Index: Makefile
===
RCS file: /home/cvs/ports/sysutils/gkrellm/plugins/reminder/Makefile,v
retrieving revision 1.23
diff -u -p -r1.23 Makefile
--- Makefile15 Sep 2017 15:37:18 -  1.23
+++ Makefile21 Jan 2022 17:59:00 -
@@ -5,7 +5,7 @@ COMMENT=GKrellM2 will remind you to do 
 V= 2.0.0
 DISTNAME=  gkrellm-reminder-${V}
 PKGNAME=   gkrellmreminder-${V}
-REVISION=  10
+REVISION=  11
 CATEGORIES=misc
 
 HOMEPAGE=  
http://members.dslextreme.com/users/billw/gkrellm/Plugins.html\#REMINDER
Index: patches/patch-reminder_c
===
RCS file: 
/home/cvs/ports/sysutils/gkrellm/plugins/reminder/patches/patch-reminder_c,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 patch-reminder_c
--- patches/patch-reminder_c3 Nov 2003 20:34:18 -   1.1.1.1
+++ patches/patch-reminder_c8 Jan 2022 10:01:06 -
@@ -1,6 +1,7 @@
 $OpenBSD: patch-reminder_c,v 1.1.1.1 2003/11/03 20:34:18 sturm Exp $
 reminder.c.orig2002-12-04 06:29:09.0 +0100
-+++ reminder.c 2003-11-02 16:42:30.0 +0100
+Index: reminder.c
+--- reminder.c.orig
 reminder.c
 @@ -87,7 +87,7 @@ static struct reminder_config {
  
  struct event_stored {
@@ -15,20 +16,22 @@ $OpenBSD: patch-reminder_c,v 1.1.1.1 200
current->name = g_strdup(buffer);
  
 -  if( fscanf( fp, "%u %d %d %ld %ld %ld\n", >id, >days,
-+  if( fscanf( fp, "%lu %d %d %d %d %d\n", >id, >days,
++  if( fscanf( fp, "%lu %d %d %lld %lld %lld\n", >id, 
>days,
>occurs, >start, >end,
>last_displayed ) != 6 )
{
-@@ -431,7 +431,7 @@ reminder_save_stored()
+@@ -431,8 +431,8 @@ reminder_save_stored()
current = head_stored;
while( current )
  {
 -  fprintf( fp, "%s\n%u %d %d %ld %ld %ld\n", current->name, current->id, 
current->days,
-+  fprintf( fp, "%s\n%lu %d %d %d %d %d\n", current->name, current->id, 
current->days,
-  current->occurs, current->start, current->end, 
current->last_displayed );
+- current->occurs, current->start, current->end, 
current->last_displayed );
++  fprintf( fp, "%s\n%lu %d %d %lld %lld %lld\n", current->name, 
current->id, current->days,
++ current->occurs, (long long)current->start, (long 
long)current->end, current->last_displayed );
  
current = current->next;
-@@ -529,7 +529,7 @@ reminder_remove_event_stored( struct eve
+ }
+@@ -529,7 +529,7 @@ reminder_remove_event_stored( struct event_stored **he
  }
  
  static struct event_stored *
@@ -63,7 +66,7 @@ $OpenBSD: patch-reminder_c,v 1.1.1.1 200
  
/* Try to remove event from temp list. If not, add to to-be-deleted list */
if( !reminder_remove_event_stored( _temp, id ) )
-@@ -1661,13 +1661,13 @@ cb_sort_days( GtkCList *clist, gconstpoi
+@@ -1661,13 +1661,13 @@ cb_sort_days( GtkCList *clist, gconstpointer *p1, gcon
  
struct event_stored *es1, *es2;
  
@@ -81,7 +84,7 @@ $OpenBSD: patch-reminder_c,v 1.1.1.1 200
  
if( es1 && es2 )
  {
-@@ -1692,13 +1692,13 @@ cb_sort_time( GtkCList *clist, gconstpoi
+@@ -1692,13 +1692,13 @@ cb_sort_time( GtkCList *clist, gconstpointer *p1, gcon
  
struct event_stored *es1, *es2;
  
@@ -99,7 +102,7 @@ $OpenBSD: patch-reminder_c,v 1.1.1.1 200
  
if( es1 && es2 )
  return( ( ( es1->start - TIMEZONE_DIFF ) % SECS_PER_DAY ) -
-@@ -1715,13 +1715,13 @@ cb_sort_start( GtkCList *clist, gconstpo
+@@ -1715,13 +1715,13 @@ cb_sort_start( GtkCList *clist, gconstpointer *p1, gco
  
struct event_stored *es1, *es2;
  
@@ -117,7 +120,7 @@ $OpenBSD: patch-reminder_c,v 1.1.1.1 200
  
if( es1 && es2 )
  return es1->start - es2->start;
-@@ -1737,13 +1737,13 @@ cb_sort_end( GtkCList *clist, gconstpoin
+@@ -1737,13 +1737,13 @@ cb_sort_end( GtkCList *clist, gconstpointer *p1, gcons
  
struct event_stored *es1, *es2;
  
@@ -144,7 +147,7 @@ $OpenBSD: patch-reminder_c,v 1.1.1.1 200
  
/* delete event from today */
num_active--;
-@@ -2914,7 +2914,7 @@ reminder_window_never( GtkWidget *window
+@@ -2914,7