On 08/27/2013 10:41 PM, Heiko Carstens wrote:
> On Tue, Aug 27, 2013 at 09:24:56AM -0400, Paul Gortmaker wrote:
>> On 13-08-26 01:02 AM, Chen Gang wrote:
>>> The 3 "if (level >=1)" are redundency, only one is enough.
>>
>> Maybe. Or maybe it is a copy and paste (vi yyp) error, and it really
>> was meant to be incremental values in each if statement.
>>
Hmm... the code itself can 'tell' us the answer.
The stsi_1*() are all for printing information, and according to their
printing contents, they are not conflict with each other, so at least,
we can permit them to print together.
And the 3 stsi_1*() are all static function which only be used within
the file, so can assume the original authors consider all of them are
useful.
And also can assume stsi_1*() match "level 1", stsi_2*() match "level
2", and stsi_3*() match "level 3" (stsi_15_* is an exception, but we can
bear -- better to get explanation from the original author).
So it is not "a copy and paste error", if it was, either at least one of
stsi_1*() would be useless or the name of stsi_1*() would not match
"level 1".
>> When you are not sure of what the code was meant to do, you should
>> ask the maintainers, vs. perhaps risking adding brokenness on top of
>> more brokenness.
>>
Hmm... in my opinion, if I am not sure of it, but can find a simple
implementation, I still want to try, so may save maintainers' time
resource (if I can not find, I will send [Suggestion] patch).
Normally, if the fix is incorrect (or need improvement), the related
maintainers can find it easily within 5 minutes (at least, they will
doubt about it), and provide valuable suggestions.
So I think this work flow is still efficient, and no negative effect
with quality.
>> Paul.
>> --
>
> The code is correct and I'd like to leave it as it is.
Yeah it is correct, but in my opinion, it can be improved.
> The plan was (and still is) to change it from a single open seqfile to one
> which actually makes use of an iterator, since the output can fill more
> than a page and I'd like to avoid reallocations and possible high-order
> allocations which might fail.
It sounds good.
> The single stsi calls are the natural granularity for iteration.
>
Yeah, so for each level (e.g. "level 1"), may also have multiple stsi
functions.
So in my opnion, it will be better to let all level 1 related functions
within "if (level >= 1) {...}" code block, that will make code clearer
to readers (or some members may doubt about this code).
BTW: could you provide more information about why need stsi_15_1_x() be
in "level 1" ?
Thanks.
--
Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-s390" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
