[ debian-hurd-Patches-303140 ] #356943 bsdmainutils
Patches item #303140, was opened at 2006-02-27 13:45
You can respond by visiting:
http://alioth.debian.org/tracker/?func=detail&atid=410472&aid=303140&group_id=30628
Category: posix
Group: submitted
>Status: Pending
Resolution: None
Priority: 5
Submitted By: Christopher Bodenstein (physicman-guest)
Assigned to: Nobody (None)
Summary: #356943 bsdmainutils
Initial Comment:
Author: Christopher Bodenstein <cb@physicman.net>
Source: bsdmainutils
Status: unreviewed
Categories: posix
Strip-Level: -p1
Just one instance of PATH_MAX to remove.
The package is available from http://packages.physicman.net/ as usual.
----------------------------------------------------------------------
Comment By: Michael Banck (mbanck)
Date: 2006-03-14 15:12
Message:
Logged In: YES
user_id=2380
<Jeroen> it looks fine
----------------------------------------------------------------------
Comment By: Christopher Bodenstein (physicman-guest)
Date: 2006-03-07 23:48
Message:
Logged In: YES
user_id=11528
Ok here is the latest instance of the patch, I hope I got it right this time ;)
----------------------------------------------------------------------
Comment By: Michael Banck (mbanck)
Date: 2006-03-07 14:25
Message:
Logged In: YES
user_id=2380
More feedback from #hurd:
<neal> Why do you free calendarpath at the end?
<neal> you allocate it at 357
<neal> also you return fd without freeing calendarpat
<neal> is calendarPath a global variable?
<neal> I vote killing the snprintf and just using sprintf
<neal> also, don't do calendarPathLength = strlen(home) + strlen(calendarHome) + strlen(calendarFile) + 3
<neal> do: calendarPathLength = strlen(home) + 1 + strlen(calendarHome) + 1 + strlen(calendarFile) + 1
<neal> this makes it more explicit that the 1's are for slashes and the trailing \0
<azeem> ah right, calendarPath is an external variable, freeing it is not what we want
----------------------------------------------------------------------
Comment By: Christopher Bodenstein (physicman-guest)
Date: 2006-03-06 10:46
Message:
Logged In: YES
user_id=11528
<azeem> Physicman: calendarPath is already declared in calendar.h or so, so I don't think we need to declare it in the patch
So here is a new version of the patch :)
----------------------------------------------------------------------
Comment By: Christopher Bodenstein (physicman-guest)
Date: 2006-03-05 12:28
Message:
Logged In: YES
user_id=11528
Attached is a new patch that takes these considerations into account.
----------------------------------------------------------------------
Comment By: Michael Banck (mbanck)
Date: 2006-03-04 16:56
Message:
Logged In: YES
user_id=2380
Uhm, some more:
<Jeroen> if you want to do snprintf anyway
<Jeroen> you should probably do something like
<Jeroen> int length = strlen(home) + strlen(calendarHome) + strlen(calendarFile) + 3;
<Jeroen> then malloc (length);
<Jeroen> and snprintf (calendarPath, length, ...);
----------------------------------------------------------------------
Comment By: Michael Banck (mbanck)
Date: 2006-03-04 16:54
Message:
Logged In: YES
user_id=2380
Some feedback from #hurd:
<Jeroen> strlen(calenderPath) is bogus
<Jeroen> strlen() gives you the length of the string, but calenderPath is just malloc'ed data, not a string
<Jeroen> in this case you can just use sprintf however
<antrik> well, I always use snprintf
<Jeroen> it's useless in this case
<antrik> you never know who is going to change the code and forget to update the length...
<Jeroen> yes
<Jeroen> but I could add few hunderds of "you never know..."
<azeem> Jeroen: so just `sprintf(calendarPath, "%s/%s/%s", home, calendarHome, calendarFile);' you mean?
<Jeroen> azeem: yes
----------------------------------------------------------------------
You can respond by visiting:
http://alioth.debian.org/tracker/?func=detail&atid=410472&aid=303140&group_id=30628
Reply to: