Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

명령어가 3개이상의 parameter를 받을 경우 ConsoleShell.c의 kGetNextParameter 함수가 비정상적으로 작동합니다. #11

Open
ybjeon01 opened this issue Aug 2, 2021 · 1 comment

Comments

@ybjeon01
Copy link

ybjeon01 commented Aug 2, 2021

안녕하세요. Ch24 공부중 pata 디스크를 2개 연결하고 readsector 명령어을 조금 수정해서 사용하다가 발견하게됬습니다. 명령어에서 디스크를 직접 선택할 수 있도록 인자를 총 4개 (is_primary, is_master, LBA, count) 받게 했는데 ConsoleShell.c의 kGetNextParameter 함수 코드가 살짝 잘못 짜여진 것을 발견했습니다. kMemCpy의 인자가 잘못되어있더라고요. 아래 코드 comment에 부가설명을 넣었습니다.

Original Code

int kGetNextParameter( PARAMETERLIST* pstList, char* pcParameter )
{
    int i;
    int iLength;

    if( pstList->iLength <= pstList->iCurrentPosition )
    {
        return 0;
    }
    

    for( i = pstList->iCurrentPosition ; i < pstList->iLength ; i++ )
    {
        if( pstList->pcBuffer[ i ] == ' ' )
        {
            break;
        }
    }
    
    // kMemCpy가 i 크기만큼 복사하는데 i가 parameter 길이보다 클 수가 있습니다.
    // 특히 명령어 인자가 3개 이상일 경우 3번째 인자만 가져야 할 pcParameter 변수가
   // 4번째 인자까지 가져가버리는 문제가 생깁니다. 그리고 4번째, 5번째... 그리고 (마지막 - 1)
   // 인자까지 같은 문제가 생기고요.
    kMemCpy( pcParameter, pstList->pcBuffer + pstList->iCurrentPosition, i );
    iLength = i - pstList->iCurrentPosition;
    pcParameter[ iLength ] = '\0';

    pstList->iCurrentPosition += iLength + 1;
    return iLength;
}

Suggested Code

int kGetNextParameter( PARAMETERLIST* pstList, char* pcParameter )
{
    int i;
    int iLength;

    if( pstList->iLength <= pstList->iCurrentPosition )
    {
        return 0;
    }
    
    for( i = pstList->iCurrentPosition ; i < pstList->iLength ; i++ )
    {
        if( pstList->pcBuffer[ i ] == ' ' )
        {
            break;
        }
    }
    
    // 제안되는 코드
    iLength = i - pstList->iCurrentPosition;
    kMemCpy( pcParameter, pstList->pcBuffer + pstList->iCurrentPosition, iLength);
    pcParameter[ iLength ] = '\0';

    pstList->iCurrentPosition += iLength + 1;
    return iLength;
}
    

혹시 괜찮다면 나중에 pull request를 넣어도 괜찮나요?

@kkamagui
Copy link
Owner

kkamagui commented Aug 3, 2021

안녕하세요 @ybjeon01 ,

감사합니다! 이건 명백한 버그군요! 제가 실수를 한 것 같습니다. 말씀하신 코드가 옳고 pull request를 주시면 제가 검토 후에 반영하겠습니다. ^^

그럼 즐거운 OS 프로그래밍 하세요 ^^)/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants