[PATCH rtems-libbsd] ppp: Fix transmitting data

[Christian MAUDERER]

Am 13.08.21 um 10:01 schrieb Chris Johns:
> On 13/8/21 5:22 pm, Christian MAUDERER wrote:
>> Hello Chris,
>>
>> Am 13.08.21 um 04:46 schrieb Chris Johns:
>>> On 12/8/21 9:42 pm, Christian Mauderer wrote:
>>>> The pppstart expected that a driver write would somehow magically
>>>> process all data passed to the write function. Because ppp disables all
>>>> buffering that originally has been in termios, that assumption is not
>>>> true for all but polled drivers.
>>>>
>>>> With this patch, the pppstart now gets and processes the feedback that
>>>> is returned from the driver via rtems_termios_dequeue_characters.
>>>>
>>>> Fixes #4493
>>>> ---
>>>>    rtemsbsd/sys/net/if_ppp.c    | 11 ++++++-----
>>>>    rtemsbsd/sys/net/if_pppvar.h |  1 +
>>>>    rtemsbsd/sys/net/ppp_tty.c   | 32 ++++++++++++++++++++++++++++----
>>>>    3 files changed, 35 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/rtemsbsd/sys/net/if_ppp.c b/rtemsbsd/sys/net/if_ppp.c
>>>> index 709f13e04..e134dc760 100644
>>>> --- a/rtemsbsd/sys/net/if_ppp.c
>>>> +++ b/rtemsbsd/sys/net/if_ppp.c
>>>> @@ -313,11 +313,12 @@ static rtems_task ppp_txdaemon(rtems_task_argument arg)
>>>>          frag=0;
>>>>            /* initialize output values */
>>>> -      sc->sc_outfcs    = PPP_INITFCS;
>>>> -      sc->sc_outbuf    = (u_char *)0;
>>>> -      sc->sc_outlen    = (short   )0;
>>>> -      sc->sc_outoff    = (short   )0;
>>>> -      sc->sc_outfcslen = (short   )0;
>>>> +      sc->sc_outfcs        = PPP_INITFCS;
>>>> +      sc->sc_outbuf        = (u_char *)0;
>>>> +      sc->sc_outlen        = (short   )0;
>>>> +      sc->sc_outoff        = (short   )0;
>>>> +      sc->sc_outoff_update = false;
>>>> +      sc->sc_outfcslen     = (short   )0;
>>>
>>> This is not the FreeBSD clang-format. Once my changes are pushed you can use the
>>> style to reformat this code.
>>
>> I touched only a small part of the file and therefore I adapted to the style in
>> that file (also it's a bit messy).
> 
> Ah ok and yes we are sort of all over the place.
> 
>> If we want the code in the ppp parts
>> reformatted, I would suggest an extra patch that applies the coding style to the
>> complete file. What do you think?
> 
> I have only just learnt about this requirement and I think it is a good one. I
> am no expert on what is the FreeBSd format so I can only highlight what I have
> observed after reformatting my code.

On the long term I would love to have automatic checks before each 
commit. But that is a topic for a CI system that we don't have.

> 
> I think it would be good to have a single pass to clean this up but it is
> painful because it breaks blame. So I do not know and happy to see what everyone
> else thinks.

As long as we don't have a script that can check the style automatically 
(CI) or at least semi-automatically (single line call before pushing 
something) I would suggest to keep the code as it is. If we do already 
have one: I'm not aware of it. So maybe we have to make it more clear 
how to use it.

As soon as that script (that does all checking with all reasonable 
exceptions for some paths) exists I would say that it is a good idea to 
do one big commit with reformattings.

Best regards

Christian

> 
> Chris
> 

-- 
--------------------------------------------
embedded brains GmbH
Herr Christian MAUDERER
Dornierstr. 4
82178 Puchheim
Germany
email: christian.mauderer at embedded-brains.de
phone: +49-89-18 94 741 - 18
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/