Richard W.M. Jones wrote:
On Fri, Jan 13, 2012 at 02:27:49PM +0800, Wanlong Gao wrote:
> Tweak the error message "e2fsck -f" and "e2fsck -fy" to
> "e2fsck-f" and "e2fsck-fy".
>
> Signed-off-by: Wanlong Gao <gaowanlong(a)cn.fujitsu.com>
> ---
> daemon/ext2.c | 20 ++++++++++++++++++--
> 1 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/daemon/ext2.c b/daemon/ext2.c
> index 79fd354..9fe938e 100644
> --- a/daemon/ext2.c
> +++ b/daemon/ext2.c
> @@ -277,9 +277,25 @@ do_resize2fs_M (const char *device)
> if (e2prog (prog) == -1)
> return -1;
>
> - r = command (NULL, &err, prog, "-M" , device, NULL);
> + r = command (NULL, &err, prog, "-M", device, NULL);
> if (r == -1) {
> - reply_with_error ("%s", err);
> + int i = 0;
> + int len = strlen (err);
Use 'size_t' (instead of 'int') for the return value of strlen.
What is 'i' used for?
> + char *err_wrap = malloc (len + 1);
> + if (!err_wrap)
> + goto err_out;
You need to call 'reply_with_error' or 'reply_with_perror' exactly
once on each error path. So here you'd need to call
reply_with_perror ("malloc");
before the 'goto'.
> + char *p = strstr (err, "e2fsck -f");
> + if (p) {
> + strncpy (err_wrap, err, p - err);
Isn't it better to use memcpy here? AFAIK using strncpy is almost
always wrong. (Jim?)
I would. The fact that strncpy does not always NUL-terminate
makes it hard to use correctly. Better to avoid it in general.
> + err_wrap[p - err] = '\0';
> + sprintf(err_wrap + (p - err), "%s%s", "e2fsck-f", p + 9);
Also, as a general rule, it is easier to code and maintain
uses of asprintf than separate malloc+sprintf.
You can depend on asprintf, since it is provided by gnulib, if needed.
And, technically sprintf can fail (with glibc, it may even invoke malloc!),
so you should handle failure.