-----Ursprüngliche Nachricht-----
Von: Lars Ellenberg <[email protected]>
Gesendet: 07.06.2010 15:34:53
An: [email protected]
Betreff: Re: [Linux-ha-dev] [PATCH] patch for ha_logger.c
>Your patch seems to change a lot of whitespace as well,
>which makes it not obvious what exactly the functional change is.
Yes, indeed.
In this case it's better to look at the changed source code as it
is about one page of source code and the target code is
much more readable (IMHO of course) as the original one.
IMHO it's also a indication if someone is able to code with a
little bit of coding style. Whitespace and indention is a part of
it. I changed cluttered tabs to spaces and changed the
different versions of whitespacing around braces.
It would be a pitty to "enhance" this piece of code with the
argument that the patch has to much difference.
The way pointer arithmetic is done is IMHO not the cleanest
and therefore defensive coding style.
>
>But thanks for pointing out the place where things could go wrong.
>I think the original intention was more like
>
>diff -r c447fc25e119 logd/ha_logger.c
>--- a/logd/ha_logger.c Fri Apr 23 19:32:43 2010 +0200
>+++ b/logd/ha_logger.c Mon Jun 07 15:30:55 2010 +0200
>@@ -105,12 +105,11 @@
> if (p + len > endp && p > buf) {
> if (LogToDaemon(priority,buf,
> strnlen(buf, 1024),FALSE)
> ==HA_OK){
>+ p = buf;
> continue;
> }else{
> return EXIT_FAIL;
> }
>- /* NOTREACHED */
>- /* p = buf; */
> }
> if (len > sizeof(buf) - 1) {
> if (LogToDaemon(priority,*argv,
>
>
>
>and the "NOTREACHED" comment added two and a half years later
>was correct, but failed to see and fix the bug instead.
>
>Not tested either, but if that on-line patch works,
>this would be the change I'd go for.
Please not with the argument that the diff is shorter. ;-)
Best regards
Andreas Mock
_______________________________________________________
Linux-HA-Dev: [email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/
