xiaoxiang781216 commented on PR #15931: URL: https://github.com/apache/nuttx/pull/15931#issuecomment-2701255545
> > @yangsong8-a1: @cederom I see what you mean. I will split the changes in 2 pr. Please help confirm whether it is appropriate to split as described below. > > ``` > > 1. drivers/serial/cdcacm: Reduce one copy of data between serial and cdcacm framework > > This PR only contains code modifications. In order to ensure that CI passes, the content in Kconfig remains unchanged. > > > > 2. [BREAKING] boards/config: remove fixed CDACM BUFFER SIZE > > Modify Kconfig and all related boards defconfig in this PR. > > ``` > > Yes exactly, thank you @yangsong8-a1 :-) This PR contains several different changes, while PR should be as small as possible and only perform single functional change, this way its better to process, validate, and in case of problems pinpoint the source and find the fix :-) > > Lets keep this PR (1) for the small change of "usb cdc acm circular buffer handling fix" (its not really reducing copy operations right?), and provide separate PR (2) for changing the usb cdc acm buffers handling (as @jerpelea suggested Documentation updates should land in a separate commit). This will keep things elegant and smooth to process / analyze :-) > > By the way are you sure that removing buffer restrictions will not allow buffer overflows or similar problems? Was the problem caused by not checking circular buffer boundaries inside the loop? If so you can reference PR(1) in the PR(2) and we can follow discussion over there :-) without the seocnd patch, the first patch can't pass ci. so, it's wrong to separate patch to the different PR. All related change should be contained in one PR, but could split to the small patch if it make the change clear. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
