On Tuesday 08 of January 2013 14:17:39 Denys Vlasenko wrote:

> On 01/07/2013 04:42 PM, Jakub Filak wrote:

> > - consider problems older than 3 days as outdated

> > - closes trac#925

> >

> > Signed-off-by: Jakub Filak <jfilak@redhat.com>

> > ---

> > src/applet/applet.c | 11 ++++++++++-

> > 1 file changed, 10 insertions(+), 1 deletion(-)

> >

> > diff --git a/src/applet/applet.c b/src/applet/applet.c

> > index 18d9c7b..4cfd1ea 100644

> > --- a/src/applet/applet.c

> > +++ b/src/applet/applet.c

> > @@ -1285,6 +1285,11 @@ int main(int argc, char** argv)

> > GList *new_dirs = NULL;

> > GList *notify_list = NULL;

> > new_dir_exists(&new_dirs);

> > +

> > + /* Age limit = now - 3 days */

> > + char min_born_time[sizeof(time_t)*3 + 1];

> > + snprintf(min_born_time, sizeof(min_born_time), "%lu", (unsigned long)(time(NULL) - 3*24*60*60));

>

> since you print it as a long, should use sizeof(long), not sizeof(time_t) above.

 

Good catch!

 

>

> > +

> > while (new_dirs)

> > {

> > struct dump_dir *dd = dd_opendir((char *)new_dirs->data, DD_OPEN_READONLY);

> > @@ -1297,7 +1302,10 @@ int main(int argc, char** argv)

> >

> > char *reported_to = dd_load_text_ext(dd, FILENAME_REPORTED_TO,

> > DD_FAIL_QUIETLY_ENOENT | DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE);

> > - if (reported_to == NULL)

> > + char *time_str = dd_load_text_ext(dd, FILENAME_TIME,

> > + DD_FAIL_QUIETLY_ENOENT | DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE);

>

> No, you don't want DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE -

 

You are right, but dd_open() fails if time element doesn't exist or if it doesn't contain valid time stamp.

Therefore time_str can't be NULL. But I'll add some check to make the code more robust.

 

>

> > +

> > + if (reported_to == NULL && strcmp(time_str, min_born_time) >= 0)

> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

> it would go BOOM here

>

> > {

> > problem_info_t *pi = problem_info_new();

> > pi->problem_dir = xstrdup((char *)new_dirs->data);

> > @@ -1315,6 +1323,7 @@ int main(int argc, char** argv)

> > notify_list = g_list_prepend(notify_list, pi);

> > }

> >

> > + free(time_str);

> > free(reported_to);

> > dd_close(dd);

> >

>

>