Bug 3700 - 2_a1: bad time argument from /etc/cron.daily/tmpwatch
Summary: 2_a1: bad time argument from /etc/cron.daily/tmpwatch
Status: RESOLVED FIXED
Alias: None
Product: Mageia
Classification: Unclassified
Component: RPM Packages (show other bugs)
Version: Cauldron
Hardware: All Linux
Priority: Normal normal
Target Milestone: ---
Assignee: Mageia Bug Squad
QA Contact:
URL:
Whiteboard:
Keywords: Junior_job, PATCH
Depends on:
Blocks:
 
Reported: 2011-12-11 06:20 CET by Bit Twister
Modified: 2012-05-26 16:21 CEST (History)
8 users (show)

See Also:
Source RPM: tmpwatch-2.10.3-1.mga2.src.rpm
CVE:
Status comment:


Attachments
Patch by Gordon Lack (880 bytes, text/plain)
2012-01-29 20:07 CET, Marja Van Waes
Details
patched /etc/sysconfig/tmpwatch (190 bytes, text/plain)
2012-03-22 16:40 CET, Bit Twister
Details
patched /etc/cron.daily/tmpwatch (460 bytes, text/plain)
2012-03-22 16:41 CET, Bit Twister
Details

Description Bit Twister 2011-12-11 06:20:09 CET
Description of problem:

bad time argument from /etc/cron.daily/tmpwatch

Version-Release number of selected component (if applicable):


How reproducible: always


Steps to Reproduce:
1. click up a terminal
2. su - root
3. /etc/cron.daily/tmpwatch

Possible solution found at https://qa.mandriva.com/show_bug.cgi?id=63122#c1
Comment 1 Manuel Hiebel 2011-12-11 11:30:04 CET
Hi, thanks for reporting this bug.
As there is no maintainer for this package I added the committers in CC.

CC: (none) => mageia, thierry.vignaud

Comment 2 Marja Van Waes 2012-01-29 20:07:49 CET
Created attachment 1458 [details]
Patch by Gordon Lack

attaching a copy of Gordon Lack's comment containing his patch from  https://qa.mandriva.com/show_bug.cgi?id=63122#c1, because I don't know what will happen to Mdv
Marja Van Waes 2012-01-29 20:08:25 CET

Keywords: (none) => PATCH
CC: (none) => marja11

Comment 3 Jeffrey Laramie 2012-03-22 12:41:14 CET
Update - This bug is still present in Beta 2.

Also, based on the documentation there are 2 different options for specifying excluded files or directories: -x which takes a path as an argument, and -X which takes a pattern. Since the argument that fails contains a wildcard I believe the correct option would be -X instead of -x, however both throw the same error.

CC: (none) => jalaramie

Thierry Vignaud 2012-03-22 13:46:12 CET

Keywords: (none) => Junior_job

Comment 4 Bit Twister 2012-03-22 16:36:46 CET
(In reply to comment #3)
> Update - This bug is still present in Beta 2.

> I
> believe the correct option would be -X instead of -x, however both throw the
> same error.

Well, yes and no. It is a two part fix.  Part 1 is:

$ dif /etc/sysconfig/tmpwatch_vorig /etc/sysconfig/tmpwatch
3c3
< TMPWATCH_EXCLUDES="-x /tmp/.ICE-unix -x /tmp/.X*-unix -x /tmp/.font-unix -x /tmp/.Test-unix"
---
> TMPWATCH_EXCLUDES="-x /tmp/.ICE-unix -X /tmp/.X*-unix -x /tmp/.font-unix -x /tmp/.Test-unix"

Part 2 is:
$ dif /etc/cron.daily/_vorig/tmpwatch_vorig /etc/cron.daily/tmpwatch
4a5
> set -f
5a7
> set +f
6a9
> set -f
7a11
> set +f
Comment 5 Bit Twister 2012-03-22 16:40:17 CET
Created attachment 1825 [details]
patched /etc/sysconfig/tmpwatch
Comment 6 Bit Twister 2012-03-22 16:41:54 CET
Created attachment 1826 [details]
patched /etc/cron.daily/tmpwatch
Comment 7 Jeffrey Laramie 2012-03-22 17:25:26 CET
(In reply to comment #4)
> (In reply to comment #3)
> > Update - This bug is still present in Beta 2.
> 
> Part 2 is:
> $ dif /etc/cron.daily/_vorig/tmpwatch_vorig /etc/cron.daily/tmpwatch
> 4a5
> > set -f
> 5a7
> > set +f
> 6a9
> > set -f
> 7a11
> > set +f

This works for me as well. Now I have to go brush up my sh scripting skills to find out why it works. ;-)
Comment 8 Juergen Harms 2012-03-28 09:24:11 CEST
Wouldnt be backslashing the * be a much simpler (and more evident) solution? - ending up with:

TMPWATCH_EXCLUDES="-x /tmp/.ICE-unix -X /tmp/.X\*-unix -x /tmp/.font-unix -x /tmp/.Test-unix"

CC: (none) => juergen.harms

Comment 9 Bit Twister 2012-03-28 14:33:25 CEST
(In reply to comment #8)
> Wouldnt be backslashing the * be a much simpler (and more evident) solution? -
> ending up with:
> 
> TMPWATCH_EXCLUDES="-x /tmp/.ICE-unix -X /tmp/.X\*-unix -x /tmp/.font-unix -x
> /tmp/.Test-unix"

I'll confirm that just changing /etc/sysconfig/tmpwatch to the above does keep /etc/cron.daily/tmpwatch from complaining. :)

I'll always vote for the "much simpler and more evident solution" every time. 8-)
Comment 10 Gordon Lack 2012-03-28 22:00:27 CEST
>> I'll always vote for the "much simpler and more evident solution" every time.

So do I.  Hence my comment in the patch about *not* begin able to do it with quoting. (i.e. That's what comments are for - making evident).

The f-flag is in set for a reason.  This is it.

You can simplify the patch to the script by removing the consecutive set +f, set -f, but only because the two tmpwatch commands there are consecutive.  This, however, would remove the symmetry of each /usr/sbin/tmpwatch invocation.

And don't try dropping it all into a function either.  What you have here is a single variable (TMPWATCH_EXCLUDES) containing multiple parameters, and one of them is a wildcard which *must not be expanded by the shell* (tmpwatch itself does the expansion).  That is an extremely fragile setup.

CC: (none) => gordon.lack

Comment 11 Jeffrey Laramie 2012-03-30 15:15:41 CEST
(In reply to comment #10)
>  That is an extremely fragile setup.

IMHO the whole configuration is a bit squirrelly...

1. I've never seen a cron script put a config file in /etc/sysconfig before. I always thought that directory was for the config files for services.

2. There are 2 variables specified in the config file; 1 is disabled and TMPWATCH_EXCLUDES only applies to one invocation of tmpwatch. I guess theoretically you could add other invocations of tmpwatch that had excluded directories, but then you would have to edit 2 files instead of simply adding the excluded directory to the new invocation.

3. tmpwatch is invoked 3 times in the cron script, but TMPWATCH_OPTIONS is only set once. What if you wanted to use different options for each invocation?

I think the 2 file setup is overly complex and less flexible than a single cron script, but maybe I just don't understand why it was done that way.
Comment 12 Gordon Lack 2012-04-01 23:18:49 CEST
/etc/sysconfig seems a reasonable place to put a config file for a *system* crontab.
This is on RedHat5:
=====
cron.daily/prelink:. /etc/sysconfig/prelink
cron.weekly/99-raid-check:# This script reads it's configuration from /etc/sysconfig/raid-check
cron.weekly/99-raid-check:[ -f /etc/sysconfig/raid-check ] || exit 0
cron.weekly/99-raid-check:. /etc/sysconfig/raid-check
=====

It's also a good idea to keep configuration separate from code - it allows you to edit a config file without worrying about code structure and also to keep code in a CMS without having to specify a config in it.

No idea what LSB says, though.
Comment 13 Jeffrey Laramie 2012-04-02 05:00:43 CEST
(In reply to comment #12)
> /etc/sysconfig seems a reasonable place to put a config file for a *system*
> crontab.
> This is on RedHat5:
> =====
> cron.daily/prelink:. /etc/sysconfig/prelink
> cron.weekly/99-raid-check:# This script reads it's configuration from
> /etc/sysconfig/raid-check
> cron.weekly/99-raid-check:[ -f /etc/sysconfig/raid-check ] || exit 0
> cron.weekly/99-raid-check:. /etc/sysconfig/raid-check
> =====
> 
> It's also a good idea to keep configuration separate from code - it allows you
> to edit a config file without worrying about code structure and also to keep
> code in a CMS without having to specify a config in it.

Oh I agree completely that config and code should be separate. In fact I have template init scripts and matching config files that put *every* variable that isn't part of the script logic in the config file, which is more than many init scripts do. It just seems in this particular case it serves no useful purpose and actually makes configuration less flexible since tmpwatch is invoked multiple times. I don't think it's a big deal worth putting a lot of thought or effort into. Just an observation.

> 
> No idea what LSB says, though.

I think LSB is a great idea and the best (only?) standard Linux has, and I support following it. (Honestly, I also think it's just sewing another arm on Frankenstein but that's a rant for another place and time)  ;-)

Jeff
Remco Rijnders 2012-04-12 07:40:42 CEST

CC: (none) => remco

Comment 14 Jeffrey Laramie 2012-05-04 02:09:13 CEST
The updated /etc/sysconfig/tmpwatch configuration file in package tmpwatch-2.10.3-2.mga2 avoids wildcards and sidesteps this issue on my system. But it also enables the first line: TMPWATCH_OPTIONS="-umc" Is that correct? Based on the man page it looks like those 3 options are mutually exclusive.

Jeff
Comment 15 Sander Lepik 2012-05-04 09:57:48 CEST
(In reply to comment #14)
> The updated /etc/sysconfig/tmpwatch configuration file in package
> tmpwatch-2.10.3-2.mga2 avoids wildcards and sidesteps this issue on my system.
> But it also enables the first line: TMPWATCH_OPTIONS="-umc" Is that correct?
> Based on the man page it looks like those 3 options are mutually exclusive.
> 
> Jeff

http://pkgs.fedoraproject.org/gitweb/?p=tmpwatch.git;a=blob;f=tmpwatch.daily;h=5ab2e7462f5bd901bdcdb19c96145d8e6bc8317e;hb=8d2580d952fa93c4f08f7bfaf53433c1ab4ecd7f

As Fedora is the upstream and they are using the same flags then i hope they know what they do. But to be sure you may ask the upstream.

CC: (none) => sander.lepik

Comment 16 Marja Van Waes 2012-05-26 13:02:07 CEST
Hi,

This bug was filed against cauldron, but we do not have cauldron at the moment.

Please report whether this bug is still valid for Mageia 2.

Thanks :)

Cheers,
marja

Keywords: (none) => NEEDINFO

Comment 17 Bit Twister 2012-05-26 16:21:04 CEST
(In reply to comment #16)

> Please report whether this bug is still valid for Mageia 2.

resolved with the patch

Keywords: NEEDINFO => (none)
Status: NEW => RESOLVED
Resolution: (none) => FIXED


Note You need to log in before you can comment on or make changes to this bug.