Re: [PATCH v4 08/25] sequencer: completely revamp the "todo" script parsing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Johannes Schindelin <johannes.schindelin@xxxxxx> writes:

> -	for (i = 1; *p; i++) {
> +	for (i = 1; *p; i++, p = next_p) {
>  		char *eol = strchrnul(p, '\n');
> -		commit = parse_insn_line(p, eol, opts);
> -		if (!commit)
> -			return error(_("Could not parse line %d."), i);
> -		next = commit_list_append(commit, next);
> -		p = *eol ? eol + 1 : eol;
> +
> +		next_p = *eol ? eol + 1 /* strip LF */ : eol;

This one was explained as "skip LF" in the previous round, and that
is more correct than "strip", I think.  The +1 here is not done to
"strip" the LF out of the end result, but to "skip" one to move to
the beginning of the next line.

The one in v3 08/25 decremented the pointer to denote the end of the
line with the explicit purpose of not including the CR in the end
result, which was explained as "skip CR", but it was stripping CR.
Correcting that explanation to "strip" was a right fix and I think
your v4 09/25 still has it, which is good.

Other than this unnecessary change since the previous round, I
didn't spot a difference in this step, which was already good.

Thanks.





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]